Skip to content

Never send resource, license, licence or values from Evidence to the cloud - #61

Merged
jwrosewell merged 7 commits into
rcfrom
evidence-reserved-keys
Nov 11, 2026
Merged

jwrosewell merged 7 commits into
rcfrom
evidence-reserved-keys

Conversation

@jwrosewell

@jwrosewell jwrosewell commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

The Cloud Request Engine now leaves out of its request any Evidence whose name after the prefix is resource, license, licence or values, in any letter case and under any prefix, as the Processing section of the Cloud Request Engine page says.

Problem

addQueryData in pipeline/fiftyone.pipeline.cloudrequestengine/cloudRequestEngine.js copied every piece of Evidence into the request body under its name after the prefix, whatever the name. On rc the engine sets its own resource, license and values fields after the Evidence, so Evidence of exactly those names in lower case is replaced. Evidence such as query.LICENSE or header.Resource still became a second field of the same name in other letters, licence was sent as it was, and values was sent whenever the engine had no property list of its own.

What the specification says

The Processing section of cloud-request-engine.md at e89679e9, the specifications rc head:

Evidence under any prefix whose key strips to values, resource, license or licence, in any letter case, MUST therefore be left out of the request, because sending it would put a second field of the same name, or a second spelling of the same credential, in the request with no way for either side to know which one applied.

and

The rule covers resource, license and licence as well as values, and does not depend on which keys the server advertises.

Change

  • addQueryData skips Evidence whose name after the prefix is one of the four names in the reservedFields constant, compared without regard to case. A key with no dot is its own name, so it is covered too, and the test pins that with a bare Licence key. The check runs before the precedence logic, so a reserved name never claims a field.
  • The engine's own resource key, license key and property list are still sent in the body, set after the Evidence, as rc already does.
  • The check does not depend on the cloud's evidencekeys answer, so it holds for every piece of Evidence the engine sends.

Tests

evidence never supplies resource, license or values in pipeline/fiftyone.pipeline.cloudrequestengine/tests/cloudRequestEngine.test.js adds query.Resource, query.LICENSE, query.Licence, query.Values, header.resource and cookie.License, plus query.other, header.other and cookie.cookie-only, and a bare Licence with no prefix, added in 3160539. It captures the POST and checks that the URL carries the resource key and no license, that the only reserved fields in the body are resource and license holding the engine's own keys, that none of the Evidence values for the reserved names appear, and that the other Evidence is sent with query. still taking precedence.

35b2e3d updates the test after rc was merged in at e1a4c6c, because rc now sends the keys in the body rather than the license key in the URL.

Every temporary run below sits in one workflow file shared by the temporary runs, so its page can carry the name of another branch in its title. Each run's own branch is named here.

  • Red, first round, the test on rc without the change: run 42721446746. Green: run 42721462583, 22 passed.
  • Red, on the current rc at c32a2905 with this draft's test file, on branch tmp/node-61-red: run 43001275649. The new test fails and the other 21 pass.
  • Green, on the head 0253418, on branch tmp/node-61-green: run 43001293870, 22 of 22 pass.
  • Full build on 0253418, success, run 43001376753. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass, 998 of 1000 tests with 2 skipped in each, Configure printed "Left out 2 macOS entries", and all 7 Setting 'PullRequestSha' lines are 0253418f9d984e715570aca53cc95c009cbf0aa6.
  • Red for the bare key, on branch tmp/node-61-red3, run 43024570384, the test at 3160539 on rc code. The test fails and the other 21 pass.
  • Full build on the head 3160539, success, run 43024622299. Configure and the Ubuntu, Ubuntu ARM and Windows jobs on Node 22 and 24 pass, 1152 of 1154 tests with 2 skipped in each, Configure printed "Left out 2 macOS entries", and all 7 Setting 'PullRequestSha' lines are 316053942bb25efcdd6368848cbcd8cc814735c7.

macOS was left out of this draft's full build under the 9 November decision. The rc head is built with macOS before it merges into main.

What remains

  • node#72 makes the engine send the Evidence no element accepts as well, and carries its own reserved name check in isSent. This draft merges first, and Send the cloud every Evidence value apart from the reserved names #72 then drops its own check in favour of this one and keeps its check for the web request view under server.http-request.
  • Python warns when it leaves a reserved name out, whilst Node and PHP leave it out without a word. The specification asks for none of the three, so that is noted here and not changed.

@jwrosewell
jwrosewell force-pushed the evidence-reserved-keys branch from f21a356 to 4293d31 Compare November 9, 2026 23:52
…es test

rc now sends the resource key and license key in the request body, set after the Evidence, so the test expects those two fields with the engine's values, no license in the URL, and no other field whose name is a reserved one.
@jwrosewell jwrosewell changed the title Send only the Evidence the cloud accepts, never resource, license or values Never send resource, license, licence or values from Evidence to the cloud Nov 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant