Conversation
A token file written with `jq -r … > file` ends in a newline, which SDKs send as-is and the signature's base64 decode rejects with a 400. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The separate refused key cost an extra Cache API match before every
lookup, and the delete of the positive entry never found anything after
a 403. A refusal is now stored as {"trusted":false} under the same key
for REFUSED_TRUST_CACHE_SECS: one Cache API read per exchange.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Platform-token refusals and the API key path's non-key errors came from build_sts_error_response without it, and SDKs show only the message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The platform path takes its issuers' tokens ahead of the STS route, so an entry for AUTH_ISSUER would refuse every person exchange. Drop it at load with an error. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fail-closed check ran before the API-key and platform short-circuits, so an empty AUTH_AUDIENCE also disabled exchanges that carry their own audience checks (platform) or none at all (keys). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Run a second worker whose AUTH_ISSUER is GitHub, so CI's token is a
person token there: tests/test_person_route.py exchanges it at
_default and signs with the result, and refuses a wrong audience and a
tampered signature. No CI test reached the person route with a
validly signed token since GitHub became a platform issuer.
- Give the main worker's AUTH_AUDIENCE the wrong-audience token's
audience, so a platform path that checked the person audiences
instead of GitHub's own would fail CI.
- The stub trusts exactly the subject of the token CI minted, as
source.coop matches a trust, instead of a repository prefix.
- Pin that a 200 saying {"trusted": false} is still refused.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With FEDERATION_TEST_AUDIENCE set but FEDERATION_TEST_TRUST_ACCOUNT unset, the token named the stub's account, which staging lacks, and the copy-source test failed instead of skipping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
🚀 Latest commit deployed to https://source-data-proxy-pr-247.source-coop.workers.dev
|
alukach
marked this pull request as ready for review
October 1, 2026 06:44
Contributor
Author
3 tasks done
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note
Stacked on #237: the new person-route tests rely on #237's rule that
AUTH_ISSUERis dropped fromPLATFORM_ISSUERS. Until #237's branch has its latest review fixes pushed, this diff also shows those five commits; the change here is the last two commits.What I'm changing
This closes test gaps found in the review of #237:
(account, issuer, subject).{"trusted": false}had no test.FEDERATION_TEST_AUDIENCEwas set. WithoutFEDERATION_TEST_TRUST_ACCOUNT, the token then named the stub's account, which staging doesn't have.How I did it
ci.ymlstarts a secondwrangler devon port 8788 with--var AUTH_ISSUER:<GitHub> --var AUTH_AUDIENCE:source-data-proxy-ci, so CI's token is a person token there.tests/test_person_route.pyexchanges it at_defaultand lists a public product with the credentials, which unseals the session. It also checks that a wrong-audience token and a tampered token are refused. That worker'sPLATFORM_ISSUERSstill names GitHub, so these tests also pin that the person route wins.AUTH_AUDIENCEis nownot-the-data-proxy, the audience of the wrong-audience token CI already mints. A platform path reading the person audiences would then accept that token and refuse the real one.subasCI_TRUSTED_SUBJECT, and the stub trusts exactly that subject.ci-tests--says-no-with-200account answers 200{"trusted": false}, and a test checks that the proxy refuses it.FEDERATION_TEST_AUDIENCEandFEDERATION_TEST_TRUST_ACCOUNTare set.Not covered: the stub still reads the proxy's assertion without verifying its signature. It has no key or crypto library, so a mis-signed trust lookup would still pass CI.
How to test it
CI on this PR is the test: the new tests need the GitHub token that only same-repository runs mint. Locally, without a token, 40 pass and 20 skip. I also checked by hand that
--varoverrides.dev.vars: with those flags, a GitHub-issued token at_defaultreaches the person route, and the worker logsPLATFORM_ISSUERS names AUTH_ISSUER; ignoring that entry.PR Checklist
Related Issues
Follows the review of #237.
🤖 Generated with Claude Code