ci: deploy with wrangler 4 and declare the rate limit as [[ratelimits]] - #245
Merged
Merged
Conversation
The KEY_EXCHANGE_LIMIT binding was declared under [[unsafe.bindings]] because the workflows pinned wrangler 3, which doesn't know the first-class [[ratelimits]] key (it needs wrangler 4.36.0 or later). Pin wrangler@4 in ci.yml, deploy.yml, preview.yml and the README, and declare the binding as [[ratelimits]] in all three environments, so wrangler validates it. The worker reads it through env.rate_limiter either way, so no code changes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
|
Claude finished @alukach's task in 10s —— View job ✅ No blocking issues — safe to merge. The diff is a mechanical change: Before the first production deploy, confirm that the Simplify (ponytail)
💰 Estimated review cost: $0.10 · 0m10s · 4 turns |
|
🚀 Latest commit deployed to https://source-data-proxy-pr-245.source-coop.workers.dev
|
3 tasks done
alukach
added a commit
that referenced
this pull request
Oct 1, 2026
) > [!IMPORTANT] > - **On `main`**, now that #235 and #236 are merged. This PR's own CI runs the integration tests that need CI's GitHub token. > - **Blocked externally for `aws-actions/configure-aws-credentials`.** That action, which source.coop's settings page hands out, checks the credentials it exports with `GetCallerIdentity`. The proxy cannot answer that call until 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 direct `AssumeRoleWithWebIdentity` call. > - **No multistore bump.** multistore 0.7.2 is enough: the one fail-closed check this needs, a required `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 its `RoleArn` names, `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. Both share one parse of the request, and the token is trimmed, so a token file ending in a newline works. 1. **Route by issuer.** The token's `iss` is read unverified. If it is in `PLATFORM_ISSUERS`, this path takes the token; otherwise the STS route does, unchanged. 2. **Body only.** As with API keys, the token must come in the form body. One in the URL is refused with `InvalidParameterValue`, because Cloudflare logs URLs and a logged token could be replayed until it expires. 3. **Check the request locally.** The Role must be one the proxy serves (#236), and `RoleArn` must name an account. That account must be a **service account**, `{owner}--{name}` as source.coop's `SERVICE_ACCOUNT_ID_REGEX` defines 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. 4. **Verify the token** against the issuer's JWKS: signature, issuer, **that issuer's own audiences**, `nbf`, and a **required** `exp`. A key id missing from the cached keys is looked up once more, so a key GitHub has just rotated in works at once. A failed or timed-out key fetch is a retryable `InternalError`, not `InvalidIdentityToken`. 5. **Ask the account.** `POST {SOURCE_API_URL}/api/v1/accounts/{account}/trusts/exchanges` with the verified `{issuer, subject}`, authenticated as the account, which is the contract on source.coop `main` (source-cooperative/source.coop#566). Per account, issuer and subject, a yes is cached for 60 seconds and a no for 10. Only a lookup the cache can't answer is charged to `STS_EXCHANGE_LIMIT`, 100 a minute per client address, shared with API-key exchanges. 6. **Mint** under the named Role, with the **account as the principal**, never the token's subject. Every refusal of the trust reads `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 to 4 costs no lookup. Every refusal at `/.sts` from the API-key and platform paths now carries the request id, and its message is XML-escaped (#246): it can echo `RoleArn` or a token's `kid`. The person route's errors still come from multistore's unescaped builder (developmentseed/multistore#160). **Configuration (#223).** `PLATFORM_ISSUERS` is a JSON object from issuer to the audiences its tokens must carry, written as a string or as a TOML table. 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. An entry for `AUTH_ISSUER` is dropped at load with an error, since it would take every person token away from the STS route. `AUTH_ISSUER` and `AUTH_AUDIENCE` still configure the person issuer, whose tokens act as their own subject and ignore `RoleArn`'s account. An empty `AUTH_AUDIENCE` now disables only the person route with 501; API keys and platform tokens still exchange. | Config | Person issuer | Platform issuers | | --- | --- | --- | | production (`wrangler.toml`) | Ory, as before | GitHub, audience `https://data.source.coop` | | staging (`[env.staging]`) | staging Ory, as before | GitHub, audience `https://data.staging.source.coop` | | previews (`wrangler.preview.toml`) | staging Ory, as before | GitHub, audience `https://data.staging.source.coop` | | CI, main worker (`ci.yml`'s `.dev.vars`) | `https://auth.example.invalid`, which mints nothing, audience `not-the-data-proxy` | GitHub, audience `source-data-proxy-ci` | | CI, second worker (port 8788) | GitHub, audience `source-data-proxy-ci` | none: the GitHub entry is dropped as `AUTH_ISSUER` | GitHub'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** - **Only a service account can be named.** The minted principal is the account segment as given, and source.coop resolves a principal as an Ory identity first. An Ory UUID fits the person and organisation handle grammar, so if a trust ever existed on such an account, platform credentials would act as a person. source.coop writes trusts only to service accounts today, so this is defense in depth, and it matches ADR-014. The `000000000000` placeholder from `_default`'s ARN is refused here too. - **A yes is cached for 60 seconds, a no for 10, under one key.** `cached_fetch` caches 200s only, and the route says no with a 403, or a 401 for an account it can't resolve. That refusal is stored as `{"trusted": false}` under the same key, through a `cache_put` extracted from `cached_fetch`. So replaying one token costs about one lookup per 10 seconds per account, issuer and subject in each data center, and a trust just added works within 10 seconds. - **The body is read as well as the status.** A 200 whose body says `trusted: false` is refused and mints nothing. It is cached like any 200, for 60 seconds; the route never sends one. - **The rate limit is charged only on a cache miss.** Anyone can mint a GitHub token for this audience, so a lookup is bounded like a flood of junk keys. #235's `KEY_EXCHANGE_LIMIT` becomes `STS_EXCHANGE_LIMIT`, one budget shared by both kinds of exchange, with the same namespace ids. A cached answer costs the Source API nothing and isn't charged, so a job matrix behind one NAT address sharing a subject stays under it. Naming a different account each time still costs a lookup and is charged. - **Key fetches are bounded and retried, not refused.** The fetch races `STS_REQUEST_TIMEOUT` (10s). An unknown key id is looked up again through a second key cache held a minute, so a forged key id costs GitHub at most one fetch a minute per isolate. After a failed fetch, multistore backs off for 30 seconds and keeps no older keys, so a GitHub outage returns 500s for that long. - **A 404 from the trusts route is a 500, not a refusal.** The route answers for any account (401 when it doesn't exist), so a 404 means the API doesn't serve the route at all. That is a deployment mismatch, the same for every account, so it reveals nothing about any one account. - **401 and 403 read the same to the caller.** source.coop answers 401 for an account that doesn't exist, and also when it can't authenticate the proxy. Both are refused like "not trusted" and cached for 10 seconds. The WARN line has the account, issuer and subject, and source.coop's log says which case it was. - **The proxy vouches for the account before it is established.** To ask the question, it signs its usual on-behalf-of assertion as the account the caller names, and sends it only to that account's trusts route. This is ADR-014's design. ADR-005 describes such assertions only for callers the proxy has already authenticated (see Docs and ADRs). - **`exp` is required here, and multistore is not bumped.** multistore 0.7.2's `verify_token` checks `exp` only when present, the exposure ADR-004's warning says must be closed before other issuers are admitted. Upstream closes it in developmentseed/multistore#146, released as 1.0.0 by developmentseed/multistore#153, not yet merged. This path needs only that one check (`platform::subject`). Bumping would also bring multistore `main`'s `BucketConfig::backend_type` 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 sets `exp`. Drop the check when the bump happens. - **CI covers both routes with GitHub's token.** On the main worker the token takes the platform path, as in production. A second worker names GitHub as `AUTH_ISSUER`, so the same token is a person token there. The main worker's `AUTH_AUDIENCE` is the audience of the wrong-audience token CI already mints: a platform path that checked the person audiences instead of GitHub's own would accept that token and refuse the real one. - **Successful exchanges are logged at INFO**, which production drops. A cached yes never reaches source.coop, so nothing durable records which workflow minted a set of credentials. Not addressed here. ## How I did it - `src/platform.rs` (new, wasm-free): - `parse_issuers`, from a JSON string or the table's object; - `unverified`, the header and claims, for routing and the key id; - `verify`, multistore-sts's `find_key` and `verify_token` against keys it is given; - `subject`, which requires `exp` and a non-empty `sub`. - `src/sts.rs`: `account(role_arn)`, the ARN's account segment, and `is_service_account_id`, source.coop's grammar without a regex crate. - `src/source_api/cache.rs`: - `get_or_fetch_trust`, through `cached_fetch` as the account, with the 10-second refusal entry; - `cached_trust`, the cache read alone, so the rate limit is charged only on a miss; - `cache_put`, extracted from `cached_fetch`. - `src/lib.rs`: - **Dispatch:** `/.sts` parses its parameters once, trims the token, and passes them to `api_key_exchange` and `platform_exchange`. The 501 for an empty `AUTH_AUDIENCE` comes after both. - **Platform path:** `platform_exchange` is the whole platform path. `platform_keys` and `fetch_keys` add the timeout and the second key cache, through `futures-util` (already in the lockfile through worker) and `worker::Delay`. - **Errors:** `sts_refusal` and `sts_error_xml` build every exchange error, escaped and with the request id. - **Rate limit:** `STS_EXCHANGE_LIMIT` replaces `KEY_EXCHANGE_LIMIT`. - `src/config.rs`: `PLATFORM_ISSUERS`, read with `var` and, for a table, `object_var`, without `AUTH_ISSUER`. `Cargo.toml`: `base64` (already in the lockfile through multistore-sts) and `futures-util`. - **Wrangler config:** - `wrangler.toml` (production and staging) and `wrangler.preview.toml` get `PLATFORM_ISSUERS` and the renamed rate-limit binding. - The binding keeps the `[[ratelimits]]` form #245 moved it to for wrangler 4, with the same namespace ids. - **README:** the variable, the binding, and a "Platform identity providers" section with the `GetCallerIdentity` caveat. - `.github/workflows/ci.yml`: - `.dev.vars` as in the table above. - The mint step exports the token's `sub` as `CI_TRUSTED_SUBJECT`. - A second `wrangler dev` runs on 8788, with its own `--host`, because `wrangler.toml`'s dev host is `localhost:8787` and SigV4 is verified against it. - `.github/workflows/staging.yml`: the dormant federation smoke test mints its token only when both `FEDERATION_TEST_AUDIENCE` and `FEDERATION_TEST_TRUST_ACCOUNT` are set. The latter is the staging service account the token acts as, which `tests/test_writes.py` reads as `CI_TRUST_ACCOUNT`. - `tests/stub_api.py`: the trusts route. - It says yes only for exactly `CI_TRUSTED_SUBJECT` on `ci-tests--github-ci`, as source.coop matches a trust exactly. - `ci-tests--says-no-with-200` answers 200 `{"trusted": false}`. - It refuses an assertion made as anyone but the account, though it reads the assertion without verifying its signature. - 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. - `tests/platform.rs`: - audiences kept per issuer; - the table form read like the string; - an issuer without an audience dropped; - unparseable config trusting none; - a JWT read unverified, and non-JWTs (an API key among them) not read; - a verified token's subject, with a missing `exp`, a missing `sub` and an empty `sub` each refused. - `tests/sts.rs`, the account segment: none for a bare name, an empty account or a truncated ARN. - `tests/sts.rs`, 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 warnings` and `cargo check --target wasm32-unknown-unknown`, through the pre-commit hook. - `pytest tests/ --ignore=tests/test_contract.py --ignore=tests/test_federation.py` against `wrangler@4 dev` and the stub, with CI's `.dev.vars`: **42 passed, 20 skipped**. These ran without a real token: - A GitHub-issuer token whose `RoleArn` names no account is refused with `InvalidParameterValue`. - A person handle, an Ory UUID or the placeholder named as the account is refused with the untrusted `AccessDenied`. The token is forged, so this proves no trusts lookup happened. - A forged GitHub token naming a service account is refused as `InvalidIdentityToken`, with the request id and no trusts lookup: the worker fetched GitHub's real JWKS, twice, and found no such key. - A platform token in the URL is refused before any trusts lookup. - A `RoleArn` containing markup and `&` comes back escaped, in a body that parses. - With `PLATFORM_ISSUERS` written as a TOML table in `wrangler.toml`, the platform tests pass as with the string. - With `AUTH_AUDIENCE` blank, a person token gets 501 and a platform token is still verified. - **With a real GitHub token**, in this PR's CI at `dc8b141`: **61 passed, 4 skipped**. Beyond `test_writes.py`'s credentialed tier, which goes through the platform path, these need the token: - A trusted workflow's credentials act as the account: the stub records the account, not the GitHub subject, as the product lookup's subject. - A token file's trailing newline is accepted. - An account that doesn't trust the workflow refuses it with the request id, and a replay within 10 seconds costs no second lookup. A 200 saying no is refused too. - A yes is cached. - On the second worker the token is a person token. It exchanges at `_default`, and the credentials sign a list. A wrong audience and a tampered signature are refused. - **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: ```yaml permissions: { id-token: write } steps: - run: | curl -sSf -H "Authorization: bearer $ACTIONS_ID_TOKEN_REQUEST_TOKEN" \ "$ACTIONS_ID_TOKEN_REQUEST_URL&audience=https://data.staging.source.coop" | jq -r .value > "$RUNNER_TEMP/token" - env: AWS_WEB_IDENTITY_TOKEN_FILE: ${{ runner.temp }}/token AWS_ROLE_ARN: arn:aws:iam::<service-account-id>:role/FullAccess AWS_ENDPOINT_URL_STS: https://<proxy>/.sts AWS_ENDPOINT_URL_S3: https://<proxy> AWS_REGION: us-west-2 run: aws s3 ls s3://<owner>/<product>/ ``` Then remove the trust, and within a minute the next exchange reads `AccessDenied … (request id …)`. ## Docs and ADRs - **ADR-009** (platform IdPs): its decision holds (platform issuers, per-issuer audiences, fail closed per issuer), and this implements it. Its migration step 3 and its important note are superseded by ADR-014: platform issuers are not added to `_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 (#232) does not list ADR-009 under **Amends**; #232 may want to. - **ADR-004** trusted "exactly one OIDC issuer" and warned that `exp` must be enforced before other issuers are admitted. A note under its trust model now points to platform issuers and ADR-014. Its `exp` warning now says platform tokens must carry `exp` and that the proxy checks it. It also says `alg` is checked before the key fetch, which is not so on the platform path: a forged header still costs one cached key lookup before `verify_token` rejects its algorithm. - **ADR-001** said `source_identity` is "the original OIDC `sub` — the caller's Ory identity". It now says it is the account an API key or a trusted platform token names, which also covers #235's keys. - **ADR-005** says the proxy signs lookups as a caller it has authenticated. The trusts lookup is signed as the account a caller names, before that account is established, and only for that one route. ADR-014 (#232) records this, and `ApiCaller::Account`'s doc now says so. ADR-005's text does not, so it is for #232 or #234 to amend. - **ADR-014** (#232), "the token path is then…", is what this implements, and it still holds. - **docs.source.coop**: no page on `main` covers GitHub Actions against the proxy. The unattended-workflow guide, source-cooperative/docs.source.coop#34, should give the token-file workflow above and the `configure-aws-credentials` caveat until developmentseed/multistore#126. - **source.coop**: `src/lib/services/github-workflow.ts` on `main` hands out `configure-aws-credentials` with `audience: <proxy origin>` and `role/FullAccess`. The audience matches `PLATFORM_ISSUERS` in production and staging, and the Role resolves since #236. The action itself waits on developmentseed/multistore#126. ## PR Checklist - [x] This PR has **no** breaking changes. Person-issuer tokens and API keys behave as before; GitHub tokens, refused until now as an untrusted issuer, take the new path. An empty `AUTH_AUDIENCE` no longer disables API-key exchange. - [x] I have updated or added new tests to cover the changes in this PR. - [x] This PR affects the [Source Cooperative Frontend & API](https://github.com/source-cooperative/source.coop): it calls the trusts route from source-cooperative/source.coop#566, already on `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. Includes #246 and #247. ADRs: #232 (ADR-014). Upstream: developmentseed/multistore#126, developmentseed/multistore#146, developmentseed/multistore#153, developmentseed/multistore#160. Epic: source-cooperative/source.coop#491. 🤖 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>
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.
What I'm changing
Follow-up to #235. The
KEY_EXCHANGE_LIMITrate limit binding is declared under[[unsafe.bindings]]because the workflows pin wrangler 3, and the first-class[[ratelimits]]key needs wrangler 4.36.0 or later. This moves the proxy to wrangler 4 and declares the binding the documented way, so wrangler validates it instead of passing anunsafeblock through unchecked.workers/public-log-streamis already on wrangler 4 (4.77.0 in its lockfile), so after this the whole repo is on v4.Decision to flag: wrangler 4 will start applying observability settings that wrangler 3 ignores. Wrangler 3 warns on the current
wrangler.tomlwithUnexpected fields found in observability field: "traces","destinations". So the Axiomdestinations = ["axiom-logs"]/["axiom-traces"]and the[observability.traces]block added in d0b46cb have probably never been applied from config, since every deploy since then went throughwrangler@3. The first wrangler 4 production deploy may start applying them. If theaxiom-logsandaxiom-tracesdestinations don't exist in the Cloudflare account, that deploy may fail; the preview and staging deploys won't catch it, because staging and preview setdestinations = []. Please confirm both destinations exist under Workers Observability → Destinations before this reaches production.How I did it
.github/workflows/ci.yml,deploy.yml,preview.ymlandREADME.md:wrangler@3→wrangler@4.wrangler.toml(production andenv.staging) andwrangler.preview.toml:[[unsafe.bindings]]→[[ratelimits]], droppingtype = "ratelimit". Name,namespace_id(1001, 1002, 1003) andsimple = { limit = 100, period = 60 }are unchanged.env.rate_limiter("KEY_EXCHANGE_LIMIT"), which doesn't care how it was declared.The pin bump and the config change have to land together. Wrangler 3 reports
Unexpected fields found in top-level field: "ratelimits"and would deploy without the binding.How to test it
wrangler deploy --dry-runwith wrangler 4.86.0 on a copy of each config (build step stubbed): production,--env stagingandwrangler.preview.toml --name …each listenv.KEY_EXCHANGE_LIMIT (100 requests/60s) Rate Limitwith no config warnings. The same production dry run on wrangler 3 shows theratelimitsand observability warnings quoted above.cargo fmt --check, clippy and check for wasm32,cargo test.wrangler devon wrangler 4 againsttests/test_api_keys.py, and this PR's preview deploy exerciseswrangler deployon v4.PR Checklist
Related Issues
Follows #235. Checked the ADRs: ADR-013 describes the per-IP limit, not how the binding is declared, so it still holds. ADR-008 says "In production, logs and traces ship to Axiom"; given the observability note above, that may not have been true while production deployed on wrangler 3, and this change makes it true, so ADR-008 needs no edit.
🤖 Generated with Claude Code