ci: run the full CI for the #235 → #236 → #237 stack (do not merge) by alukach · Pull Request #239 · source-cooperative/data.source.coop

alukach · 2026-09-25T21:40:44Z

Do not merge. This PR exists only to run CI and will be closed when it reports.

CI (.github/workflows/ci.yml) runs only for pushes and pull requests into main, so the stacked PRs #236 (base feat/opaque-api-keys) and #237 (base feat/named-roles) get no integration tests. That includes #237's tests of the platform-trust path with a real GitHub Actions OIDC token, which run only here, on a same-repo pull request into main. This PR points #237's head, the top of the stack, at main, so one run covers #235, #236 and #237 together. Review each change in its own PR, not here.

Expected: the Security Audit check fails on main's lockfile until #238 lands; everything else should pass.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd

ADR-013 as revised (#234): a service account's API key is an opaque `sck_` secret that source.coop stores as a SHA-256 hash. When `/.sts` receives one as `WebIdentityToken`, the proxy trims and format-checks it locally, hashes it, and asks `POST {SOURCE_API_URL}/api/v1/service-account-keys/exchanges` whether it is active and for which account, authenticated as itself with the sentinel subject `urn:source:data-proxy`. The answer is cached for 60 seconds, inactive answers included; an API failure fails closed with a 500 and caches nothing. It then mints credentials under the `_default` role for the account the API names, through the STS crate's minting and sealing. A key is accepted only from a POST form body. One in the query string is refused before any lookup, with a message saying why, because Cloudflare logs request URLs. Every refusal of the key itself reads "API key was not accepted (request id …)" with the id also in `x-amzn-requestid`; the reason is logged once under that id. Attempts are rate-limited per client IP by a new `KEY_EXCHANGE_LIMIT` ratelimit binding in every wrangler config; a deployment without it logs an error and does not refuse traffic. `ApiAuth` gains `authorization_header_as_self` and refuses the sentinel as an on-behalf-of subject; `cached_fetch` takes a method, an optional JSON body and an `ApiCaller`. `sts::default_role` is factored out so the key exchange mints under the same role. Supersedes #233: nothing signs a key, so `/.keys`, self-verification against the proxy's own JWKS, the `api_key` role and the multistore pin are gone. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd

`/.sts` now resolves `RoleArn` to one of three hardcoded Roles, for ID tokens and API keys alike: `FullAccess`, the unlimited Role that `_default` has been; `ReadOnly`, whose sealed ceiling allows only reads; and `_default`, kept as an alias of `FullAccess` because deployed clients name it. Each is accepted bare or as the `role/<name>` resource of an ARN of any partition and account, since SDKs insist on an ARN. Any other name is `RoleNotFound`, never a fallback to a default. ReadOnly's ceiling rides in the session token's `allowed_scopes` as one scope over every product (`*`) that lists the read actions. multistore's own scope check never runs on this gateway, so the registry enforces it: `get_bucket` checks the ceiling before anything is fetched and refuses with the same `AccessDenied` as every other refusal (ADR-011). The ceiling only subtracts, so a write it allows still needs the account's write permission. No scopes means no ceiling, which keeps every `_default` session already issued working unchanged. source.coop's GitHub integration snippet already names `role/FullAccess`; this is what makes that name resolve. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd

ADR-004 described a single built-in `_default` Role, and ADR-001 said the sealed `allowed_scopes` was always empty and consulted nowhere, with empty meaning deny-all wherever scopes are evaluated. Both are dated by the hardcoded `FullAccess` and `ReadOnly` Roles: ADR-004 gains a note pointing to ADR-014, and ADR-001 now says the bucket registry enforces the ceiling and reads an empty one as no ceiling. ADR-011's status records that its step 2 and denial semantics are implemented for `ReadOnly`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd

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

claude · 2026-09-25T21:43:05Z

Claude finished @alukach's task in 3m 13s —— View job

Review

Gather context (diff vs origin/main, read changed files)
Review for correctness bugs, security issues, best-practice problems
Apply ponytail-review pass for over-engineering
Post final findings

✅ No blocking issues — safe to merge.

This PR is CI-only plumbing plus the squashed contents of #235 (Named Roles), #236 (opaque API keys), and #237 (platform-IdP trust). Per the PR description, reviewed the diff anyway since it's the only place the platform-trust path gets exercised against a real GitHub OIDC token.

Focused on the trust-critical paths in src/lib.rs (platform_exchange/exchange_platform_token, api_key_exchange), src/platform.rs, src/keys.rs, src/sts.rs, and src/source_api/cache.rs:

Issuer routing is safe: platform::unverified() reads iss from an unverified token only to pick which issuer's JWKS to fetch (config.platform_issuers is an admin-configured allowlist, not attacker input), and platform::verify() still checks the signature against that issuer's real keys — a forged iss claim can't produce a valid signature. No SSRF, since the issuer must already be a key in the allowlist.
Ordering is deliberate and correct: RoleArn account-name + is_service_account_id check happens before signature verification (cheap local rejection first), verification happens before the Source API trust lookup, and only the trust lookup (the one call that costs the Source API something) is gated by within_rate_limit (src/lib.rs:588-600 area). All refusal paths return the same generic AccessDenied/Not authorized... message regardless of why, avoiding an oracle.
authz::ceiling_permits runs before any product fetch in registry.rs, and tests/authz.rs's read_only_allows_exactly_the_reads cross-checks it against is_write_action so the two can't drift apart.
source_api/cache.rs: only 200 responses are cached by cached_fetch; the trust route's 403 is cached separately, under its own key, with a much shorter TTL (REFUSED_TRUST_CACHE_SECS) — no accidental caching of denials at the long TTL.
ApiAuth::authorization_header refuses to sign an on-behalf-of assertion for PROXY_SELF_SUBJECT, so nothing can trick the proxy into forwarding a caller-supplied subject as itself.
Test coverage is solid on both sides (tests/platform.rs, tests/keys.rs, tests/test_platform_trust.py, tests/test_api_keys.py), including the "forged token costs zero trust lookups" and "cached refusal costs zero lookups" cases.

Nothing here rises to blocking. Two non-blocking observations:

src/lib.rs: exchange_platform_token and cached_fetch both carry #[allow(clippy::too_many_arguments)] (8 params each). Not urgent, but if either grows another parameter, bundling the issuer/audiences/duration trio (or method/body/ttl trio) into a small struct would read better than adding a 9th positional arg.
src/keys.rs/src/platform.rs: the very dense doc-comment style (explaining why down to sentence fragments) is heavier than typical Rust doc comments, but it matches this repo's existing ADR-heavy convention, so leaving as-is rather than flagging as a nit.

Simplify (ponytail): nothing to cut — key_hash's manual hex-encode and is_service_account_id's hand-rolled char-class check are each ~3 lines and avoid pulling in a hex or regex dependency for a one-off check, which is the right call here.

💰 Estimated review cost: $0.77 · 3m13s · 28 turns

github-actions · 2026-09-25T21:48:03Z

🚀 Latest commit deployed to https://source-data-proxy-pr-239.source-coop.workers.dev

Date: 2026-09-25T21:51:56Z
Commit: e3a5144

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

alukach · 2026-09-25T21:56:56Z

Done: CI ran the full stack (#235 → #236 → #237) at ba94fbc with a real GitHub OIDC token. Integration tests: 54 passed, 4 skipped, including every platform-trust and API-key test. The only failure is Security Audit, which is main's lockfile and is fixed by #238. Closing; this PR was never meant to merge.

alukach · 2026-09-25T23:03:01Z

Reopened to run the full CI once more: the stack (#235 → #236 → #237) was rebased onto main after #238 merged. Will close again when it reports.

alukach and others added 6 commits September 25, 2026 13:35

alukach deployed to preview September 25, 2026 21:42 — with GitHub Actions Active

source-release-bot Bot added the ci label Sep 25, 2026

alukach deployed to preview September 25, 2026 21:47 — with GitHub Actions Active

alukach deployed to preview September 25, 2026 21:52 — with GitHub Actions Active

alukach deployed to preview September 25, 2026 21:53 — with GitHub Actions Active

alukach mentioned this pull request Sep 25, 2026

feat(sts): let platform IdP tokens act as accounts that trust them #237

Open

3 tasks done

alukach closed this Sep 25, 2026

alukach mentioned this pull request Sep 25, 2026

ci: run the full CI for the rebased #235 → #236 → #237 stack (do not merge) #240

Closed

Navigation Menu

ci: run the full CI for the #235 → #236 → #237 stack (do not merge) - #239

ci: run the full CI for the #235 → #236 → #237 stack (do not merge)#239
alukach wants to merge 7 commits into
mainfrom
feat/platform-trust

alukach commented Sep 25, 2026

Uh oh!

claude Bot commented Sep 25, 2026 •

edited by github-actions Bot

Loading

Uh oh!

github-actions Bot commented Sep 25, 2026 •

edited

Loading

Uh oh!

alukach commented Sep 25, 2026

Uh oh!

alukach commented Sep 25, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

alukach commented Sep 25, 2026

Uh oh!

claude Bot commented Sep 25, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review

Uh oh!

github-actions Bot commented Sep 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alukach commented Sep 25, 2026

Uh oh!

alukach commented Sep 25, 2026

Uh oh!

This branch was successfully deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

claude Bot commented Sep 25, 2026 •

edited by github-actions Bot

Loading

github-actions Bot commented Sep 25, 2026 •

edited

Loading