Conversation
|
Claude finished @alukach's task in 25s —— View job ✅ No blocking issues — safe to merge. I read the Rust changes (
Non-blocking notes
Simplify (ponytail)
💰 Estimated review cost: $0.22 · 0m24s · 9 turns |
|
🚀 Latest commit deployed to https://source-data-proxy-pr-237.source-coop.workers.dev
|
## What I'm changing `cargo audit` fails on `main`, and so the Security Audit check fails on every open PR, including the machine-identity PRs (#232, #235). The cause is a new advisory against rustls 0.23.42, [RUSTSEC-2026-0285](https://rustsec.org/advisories/RUSTSEC-2026-0285): TLS 1.3 handshake messages were accepted across encryption-level boundaries. It is patched in 0.23.45. This bumps the lockfile to it. ## How I did it `cargo update -p rustls --precise 0.23.45`. A plain `cargo update -p rustls` stops at 0.23.43; the precise bump moves aws-lc-rs to 1.18.1, aws-lc-sys to 0.45.0 and rustls-webpki to 0.103.15 with it. `Cargo.lock` only. rustls never reaches the Worker. `cargo tree -i rustls --target wasm32-unknown-unknown` prints nothing; natively it comes in through multistore → reqwest → hyper-rustls, which the native tests use. Production was not exposed. ## How to test it - `cargo audit`: no vulnerabilities, with the one allowed warning (chacha20) CI already allows. - The pre-commit hook: `cargo fmt --check`, `cargo clippy --target wasm32-unknown-unknown -- -D warnings`, `cargo check --target wasm32-unknown-unknown` and `cargo test`, all passing. ## PR Checklist - [x] This PR has **no** breaking changes. - [x] I have updated or added new tests to cover the changes in this PR. (None apply: lockfile only.) - [x] This PR does not affect the Source Cooperative Frontend & API. ## Related Issues Unblocks the Security Audit check on #232, #235 and the PRs stacked on #235 (#236, #237); their pull-request runs check out the merge with `main`, so a re-run passes once this lands. #220 would catch the next one on a schedule. Part of source-cooperative/source.coop#491 only in that it clears CI for it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
ba94fbc to
12d784d
Compare
12d784d to
e1b6959
Compare
e1b6959 to
f10d12a
Compare
4234ceb to
6140cf8
Compare
A token from a platform issuer, GitHub Actions to begin with, now acts at `/.sts` as the account its `RoleArn` names (`arn:aws:iam::<account>:role/FullAccess`), and only if that account trusts the token's issuer and subject (ADR-014). The proxy reads the token's issuer unverified to route it, checks the Role and that `RoleArn` names an account, then verifies the token against the issuer's JWKS with that issuer's own audiences and a required `exp`, since multistore checks `exp` only when the claim is present. Only then does it ask `POST /api/v1/accounts/{account}/trusts/exchanges` with the verified issuer and subject, as the account. A yes is cached for 60 seconds per account, issuer and subject. A no, the route's 403, is not cached, so a trust just added works on the next attempt. The credentials' principal is the account, never the token's subject, and every refusal of the trust reads `AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …)`.
Platform issuers are configured in `PLATFORM_ISSUERS`, a JSON object from issuer to the audiences its tokens must carry, so that one issuer's audience never admits another's token (ADR-009); an issuer with no audience is refused. `AUTH_ISSUER` and `AUTH_AUDIENCE` still configure the person issuer, whose tokens act as their own subject as before. Production trusts GitHub with the proxy's origin as the audience, which is how source.coop's workflow snippet mints the token; staging and previews use staging's origin.
CI now configures GitHub as a platform issuer, as production does, and the stub answers the trusts route: yes for this repository's workflows on one account, no otherwise. The credentialed write tests name that account.
`aws-actions/configure-aws-credentials` still fails after a successful exchange, on `GetCallerIdentity`, until developmentseed/multistore#126 lands.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd
ADR-009's migration planned to add platform issuers to `_default` and warned that a CI token would then act as its subject's whole account. ADR-014's trust path replaces that: a platform token acts only as an account that trusts its issuer and subject. ADR-009 gains a note saying so, and its status now reads implemented in part. ADR-004 now notes platform issuers under its trust model, and its `exp` warning says platform tokens must carry the claim, which the proxy checks itself. ADR-001's `source_identity` now says it is the account an API key or a trusted platform token names, never a platform token's subject. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd
`get_or_fetch_trust` said only a yes is cached. `cached_fetch` caches every 200, so a `200 {"trusted": false}`, which the route does not send today, would be cached too, and still refused. The comments now say that the route's yes is a 200 cached like any other and its no is a 403 that never is.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd
## What I'm changing
ADR-013 now specifies an API key as `sck_`, 30 random base62 characters
and a six-character checksum: a fixed 40 characters matching
`^sck_[0-9A-Za-z]{36}$`, in place of `sck_` and 43 base64url characters
with no checksum. The checksum is the CRC-32 of the 30 random characters
(IEEE, as zlib computes it), written in base62 with the digits
`0-9A-Za-z`, most significant first, padded with `0` to six. This is
GitHub's own token layout.
## Why
GitHub's [secret-scanning partner
program](https://docs.github.com/en/code-security/tutorials/secret-scanning-partner-program#identify-your-secrets-and-create-regular-expressions)
recommends three things for a secret format: a unique prefix, high
entropy, and a 32-bit checksum. The ADR already had the first two. The
checksum lets a scanner, the proxy or the CLI tell a real key from a
look-alike, a truncated key or a mistyped one without asking
source.coop, and so gives a mistyped key a refusal of its own. It adds
no security, since anyone can compute it; the ADR says so. Base62 keeps
`-` out of the key, so a double-click selects all of it: about half of
the base64url keys contained one.
The random part is 178 bits, down from 256; the ADR's "no salt or KDF"
and "enumeration is infeasible" arguments still hold at that size, and
the text now says 178.
The proxy's step 2 gains the checksum check and the distinct refusal,
"API key is malformed; check that it was copied whole", which reveals
nothing because the format is public.
ADR-013 is revised in place, as it was on 2026-09-25, because nothing
implementing it has shipped: source.coop issues keys since
source-cooperative/source.coop#570 merged, but no deployed proxy can
exchange one until #235 lands.
## Implementing PRs
- #235 checks the checksum at `/.sts`
(commits d6745e0 and 30ad22f); #236 and #237 are rebased onto it.
- source-cooperative/source.coop#596 generates keys in the new format
and shows the checksum as a key's hint;
source-cooperative/source.coop#581, stacked on it, validates leaked keys
by checksum. What to send GitHub, and the equivalent steps for other
scanners, are logged on source-cooperative/source.coop#561.
- source-cooperative/source-coop-cli#20 checks a key file's format and
checksum before exchanging it.
## Docs and ADRs
Only ADR-013 states the key format; I checked the other ADRs with `git
grep sck_` and none mention it. ADR-014's amendment of ADR-013 is about
ownership, not format, and still holds.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
A platform token's `RoleArn` must now name a service account, `{owner}--{name}` as source.coop's `SERVICE_ACCOUNT_ID_REGEX` defines it (82 characters at most). Anything else is refused as an untrusting account is, before the token is verified and before the proxy signs anything as it. The minted principal is that segment as given, and source.coop resolves a principal as an Ory identity first; an Ory id fits the person and organisation handle grammar, so without this, a trust on such an account would let platform credentials act as a person. source.coop writes trusts only to service accounts today, and ADR-014 makes trusts a service account's, so this is defense in depth.
Anyone can mint a GitHub token for the proxy's audience in their own workflow, and a refusal was not cached, so replaying one valid token cost a trusts lookup per request. Two bounds follow. The per-address rate limit from the API-key exchange now also covers platform exchanges, after everything local and before the trusts lookup; the binding is renamed `STS_EXCHANGE_LIMIT`, since it now bounds every exchange that costs a Source API call, and stays at 100 a minute. And a refusal from the trusts route is cached for 10 seconds under its own key, so a replay flood from many addresses costs about one lookup per 10 seconds per account, issuer and subject, while a trust just added still works within seconds.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd
6140cf8 to
aded1ac
Compare
|
Rebased onto |
Hold a 200 trusted:false answer for REFUSED_TRUST_CACHE_SECS, as a 403 is, instead of the 60s positive TTL. Bundle the unverified token's parts into PlatformToken so exchange_platform_token no longer needs clippy::too_many_arguments. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Important
main, now that feat(sts): exchange opaque API keys at /.sts by hash lookup #235 and feat(sts): serve the FullAccess and ReadOnly roles #236 are merged; four commits. Before they merged, the integration tests that need CI's GitHub token ran on do-not-merge PRs from this branch intomain, ci: run the full CI for the #235 → #236 → #237 stack (do not merge) #239 and then ci: run the full CI for the rebased #235 → #236 → #237 stack (do not merge) #240, both closed; they now run on this PR directly.aws-actions/configure-aws-credentials. That action, which source.coop's settings page hands out, checks the credentials it exports withGetCallerIdentity. The proxy cannot answer that call until feat(sts): GetCallerIdentity + real configure-aws-credentials integration test developmentseed/multistore#126 lands, so the action fails after a successful exchange. An AWS SDK's own web-identity provider works now, as does any directAssumeRoleWithWebIdentitycall.exp, is made here. See "Decisions to flag".What I'm changing
A token from a platform issuer, GitHub Actions to begin with, can now be exchanged at
/.sts. It acts as the service account itsRoleArnnames,arn:aws:iam::<owner>--<name>:role/FullAccess, and only if that account trusts the token's issuer and subject (ADR-014). The path runs ahead of the STS route, next to the API-key exchange:issis read unverified. If it is inPLATFORM_ISSUERS, this path takes the token; otherwise the STS route does, unchanged.RoleArnmust name an account. That account must be a service account,{owner}--{name}as source.coop'sSERVICE_ACCOUNT_ID_REGEXdefines it (82 characters at most). Any other account is refused as an untrusting one is, before the token is verified or anything is signed as it.nbf, and a requiredexp.STS_EXCHANGE_LIMIT, 100 a minute per client address, is feat(sts): exchange opaque API keys at /.sts by hash lookup #235's API-key limit renamed. It now covers every exchange that may cost a Source API call.POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchangeswith the verified{issuer, subject}, authenticated as the account, which is the contract on source.coopmain(feat(accounts): trust subjects per account, the way a role's trust policy does source.coop#566). Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10.AccessDenied: Not authorized to perform sts:AssumeRoleWithWebIdentity (request id …), and the proxy logs the account, issuer and subject at WARN.Nothing unverified reaches the Source API: a token that fails steps 2 or 3 costs no lookup.
Configuration (#223).
PLATFORM_ISSUERSis a JSON object from issuer to the audiences its tokens must carry. The audiences are per issuer, so one issuer's audience never admits another's token (ADR-009). An issuer with no audience is refused, and a value that doesn't parse trusts none.AUTH_ISSUERandAUTH_AUDIENCEare unchanged: they configure the person issuer, whose tokens still act as their own subject and ignoreRoleArn's account.wrangler.toml)https://data.source.coop[env.staging])https://data.staging.source.coopwrangler.preview.toml)https://data.staging.source.coopci.yml's.dev.vars)https://auth.example.invalid, which mints nothingsource-data-proxy-ciGitHub's audience is the proxy's origin because source.coop's workflow snippet mints the token with
audience: <proxy origin>. A preview's hostname changes per PR, so previews accept staging's origin, as they already accept staging's Ory clients.Why one PR for #222 and #223. The trust check (#222) is reachable only once a second issuer is configured (#223). The configuration is safe only with the trust check: without it, a GitHub token would act as its own subject under an unlimited Role, the risk ADR-009's important note describes. Neither is useful or safe alone.
#222's issue body is superseded by its later comment, the source-cooperative/source.coop#566 contract. The body asked for an issuer-qualified principal. Under the comment's contract, a platform token's subject never becomes a principal at all: the principal is the account that trusts it. The trust answer is cached per issuer, so two issuers' identical subjects cannot share an entry, which is what #222's "done when" asks.
Decisions to flag
000000000000placeholder from_default's ARN is refused here too.cached_fetchcaches 200s only, and the route says no with a 403, or a 401 for an account it can't resolve. That refusal is stored next to the positive entry, under its own key, through acache_putextracted fromcached_fetch. So replaying one token from many addresses costs about one lookup per 10 seconds per account, issuer and subject, and a trust just added works within 10 seconds.trusted: falseis refused and mints nothing. It would also be cached like any 200, as the keys route's refusals are, so it fails closed if the route ever moved to an always-200 contract like the keys route.expis required here, and multistore is not bumped. multistore 0.7.2'sverify_tokenchecksexponly when present, the exposure ADR-004's warning says must be closed before other issuers are admitted. Upstream closes it in feat(sts)!: fail closed on empty trust fields, check token type, require exp, log successes developmentseed/multistore#146, released as 1.0.0 by chore(main): release 1.0.0 developmentseed/multistore#153, not yet merged. This path needs only that one check (platform::subject). Bumping would also bring multistoremain'sBucketConfig::backend_typeenum (refactor(core): make BucketConfig::backend_type a typed BackendType enum developmentseed/multistore#155) into the registry for nothing this PR needs. The person path keeps 0.7.2's behaviour, which is harmless while Ory always setsexp. Drop the check when the bump happens.KEY_EXCHANGE_LIMITbecomesSTS_EXCHANGE_LIMIT, one budget of 100 a minute per client address shared by both kinds of exchange; a job exchanges once. It applies after the local checks and verification, and before the trusts lookup. The throttling message now reads "too many exchanges from this address" for both kinds. The namespace ids are unchanged.arn:aws:iam::<account>:role/<role>breaking the XML.How I did it
src/platform.rs(new, wasm-free):parse_issuers,unverified(the header and claims, for routing and the key id),verify(multistore-sts'sfind_keyandverify_token), andsubject(requiresexpand a non-emptysub).src/sts.rs:account(role_arn), the ARN's account segment, andis_service_account_id, source.coop's grammar without a regex crate.src/source_api/cache.rs:get_or_fetch_trust, throughcached_fetchas the account, with the 10-second refusal entry, andcache_put, extracted fromcached_fetch.src/lib.rs:platform_exchangeandexchange_platform_token, which returns the refusal response itself.with_request_idandthrottledare shared with the key exchange, andSTS_EXCHANGE_LIMITreplacesKEY_EXCHANGE_LIMIT.src/config.rs:PLATFORM_ISSUERS.Cargo.toml:base64, already in the lockfile through multistore-sts.wrangler.toml(production and staging) andwrangler.preview.toml:PLATFORM_ISSUERS, and the rate-limit binding renamed. The binding keeps the[[ratelimits]]form ci: deploy with wrangler 4 and declare the rate limit as [[ratelimits]] #245 moved it to for wrangler 4, with the same namespace ids.README.md: the variable, the binding, and a "Platform identity providers" section with theGetCallerIdentitycaveat..github/workflows/ci.yml:.dev.varsas in the table above..github/workflows/staging.yml: the dormant federation smoke test now also needsFEDERATION_TEST_TRUST_ACCOUNT, the staging service account its token acts as.tests/test_writes.pyreads it asCI_TRUST_ACCOUNT, falling back to the stub's account.tests/stub_api.py: the trusts route. It says yes only for this repository's GitHub subjects onci-tests--github-ci, and refuses an assertion made as anyone but the account. It also keeps a per-account lookup counter and records who each product was looked up as.How to test it
cargo test: all suites pass. The newtests/platform.rscovers audiences kept per issuer, an issuer without an audience dropped, unparseable config trusting none, a JWT read unverified, non-JWTs (an API key among them) not read, and a verified token's subject, with a missingexp, a missingsuband an emptysubeach refused.tests/sts.rscovers the account segment, and no account for a bare name, an empty account or a truncated ARN. It also covers the service-account grammar: ids accepted up to 82 characters, and refused for handles, an Ory UUID, the placeholder, uppercase, underscores, one-character halves, a triple hyphen, a second separator, edge hyphens and 83 characters.cargo fmt --check,cargo clippy --target wasm32-unknown-unknown -- -D warningsandcargo check --target wasm32-unknown-unknown, through the pre-commit hook.pytest tests/ --ignore=tests/test_contract.pyagainstwrangler dev(wrangler 3.114) and the stub, with CI's new.dev.vars: 40 passed, 18 skipped. These ran without a real token:RoleArnnames no account is refused withInvalidParameterValue.AccessDenied, even for a forged token, and no trusts lookup reaches the stub.InvalidIdentityToken, with no trusts lookup: the worker fetched GitHub's real JWKS and found no such key.With a real GitHub token, on ci: run the full CI for the #235 → #236 → #237 stack (do not merge) #239 (CI's integration job at
ba94fbc): 54 passed, 4 skipped. That includes the three tests that need the token, andtest_writes.py's credentialed tier, which now goes through this path.End to end, after deploy, against staging or this PR's preview, both of which accept staging's audience: give a staging service account a trust for a workflow's subject in source.coop, then run in that workflow:
Then remove the trust, and within a minute the next exchange reads
AccessDenied … (request id …).Docs and ADRs
_default, and a platform token acts only as an account that trusts it. A note there now says so, and its status says implemented in part. ADR-014 (docs(adr): ADR-014 service accounts; amend ADR-010 and ADR-013 #232) does not list ADR-009 under Amends; docs(adr): ADR-014 service accounts; amend ADR-010 and ADR-013 #232 may want to.expmust be enforced before other issuers are admitted. A note under its trust model now points to platform issuers and ADR-014. Itsexpwarning now says platform tokens must carryexpand that the proxy checks it.source_identityis "the original OIDCsub— the caller's Ory identity". It now says it is the account an API key or a trusted platform token names, which also covers feat(sts): exchange opaque API keys at /.sts by hash lookup #235's keys.maincovers GitHub Actions against the proxy. The unattended-workflow guide, Unattended workflow guide docs.source.coop#34, should give the token-file workflow above and theconfigure-aws-credentialscaveat until feat(sts): GetCallerIdentity + real configure-aws-credentials integration test developmentseed/multistore#126.src/lib/services/github-workflow.tsonmainhands outconfigure-aws-credentialswithaudience: <proxy origin>androle/FullAccess. The audience matchesPLATFORM_ISSUERSin production and staging, and the Role resolves since feat(sts): serve the FullAccess and ReadOnly roles #236. The action itself waits on feat(sts): GetCallerIdentity + real configure-aws-credentials integration test developmentseed/multistore#126.PR Checklist
main.Related Issues
Closes #222. Part of #223: this is the proxy side, and #223's "done when", a workflow in an unrelated repository writing with only its ambient token, also needs a deployment and, for the action source.coop hands out, developmentseed/multistore#126. Builds on #235 and #236, both merged. ADRs: #232 (ADR-014). Upstream: developmentseed/multistore#126, developmentseed/multistore#146, developmentseed/multistore#153. Epic: source-cooperative/source.coop#491.
🤖 Generated with Claude Code
https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd