Skip to content

feat(sts): serve the FullAccess and ReadOnly roles - #236

Merged
alukach merged 2 commits into
mainfrom
feat/named-roles
Sep 30, 2026
Merged

alukach merged 2 commits into
mainfrom
feat/named-roles

Conversation

@alukach

@alukach alukach commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

On main, now that #235 is merged; two commits. How to test it lists what I ran locally.

What I'm changing

/.sts serves three hardcoded Roles instead of one, for ID tokens and API keys alike, as ADR-014 (#232) specifies:

Role Credentials may
FullAccess do everything the account's memberships allow
ReadOnly do the same, except write
_default do what FullAccess does; kept because deployed clients name it
  • Names. Each Role is accepted bare or as the role/<name> resource of an ARN of any partition and account (arn:aws:iam::000000000000:role/ReadOnly), because SDKs check the ARN shape before sending. A pathed resource (role/team/ReadOnly), another case (readonly) or any other name is RoleNotFound, never a fallback to a default.
  • The ceiling. ReadOnly seals one scope into the session token's allowed_scopes: every product (*), read actions only (GetObject, HeadObject, ListBucket). The gateway never calls multistore's own scope check (auth::authorize), because this proxy's registry is the authorizer, so the registry enforces it. get_bucket checks the ceiling first, before any Source API lookup, and refuses with the same AccessDenied as every other refusal (ADR-011, Denial Semantics). An INFO log line is the only record of why.
  • It only subtracts. A write the ceiling allows still needs the account's own write permission, fetched as before.
  • No scopes, no ceiling. FullAccess and _default seal no scopes, as _default never has, so every session already issued keeps working. A _default exchange returns the same response as before, AssumedRoleId included.
  • Both entry points. The STS route (StsCredentialRegistry::get_role) and the API-key exchange (exchange_api_key, which accepted only _default) share one lookup, sts::role. The key exchange's log line now names the Role.

source.coop main's GitHub integration snippet (src/lib/services/github-workflow.ts) already hands out arn:aws:iam::<service-account-id>:role/FullAccess. This PR makes that Role name resolve. The rest of that path is the next PR in this stack, which trusts a GitHub token for the account it names (#222, #223), plus GetCallerIdentity in developmentseed/multistore#126.

Decisions to flag

  • The first comment on Add the ReadOnly Role alongside FullAccess #221 is superseded. It says ReadOnly cannot be enforced until multistore plumbs the assumed Role through to the registry. That isn't needed: what is sealed is the Role's ceiling, not its name, and AuthenticatedIdentity.allowed_scopes already reaches BucketRegistry::get_bucket (multistore 0.7.2, auth/identity.rs). No multistore change.
  • The ceiling understands only what the two Roles need: a scope over every product (*) with no prefix. A scope naming one product or a prefix permits nothing rather than being half-interpreted. ADR-011's resource matching can arrive with account-owned Roles. The * sentinel means something only to this proxy, because multistore's exact-match authorize never runs here.
  • Empty scopes mean no ceiling in the registry. This is the reverse of multistore's authorize, where empty means deny-all, as ADR-001 noted. It is safe because only this proxy mints session tokens, sealed with SESSION_TOKEN_KEY, and every _default session carries empty scopes.
  • ReadOnly excludes GetObjectVersion (reading a versioned copy source). The write gate already classes it as a write, and a native test pins that the ceiling and is_write_action agree on every action. That is the divergence ADR-011 warns about.
  • ARNs are now parsed as six colon-separated fields with a role/<name> resource. The old _default check matched only the arn: prefix and the :role/_default suffix, so it also accepted malformed strings such as arn:role/_default. Those are now refused, and no SDK sends them anyway: they fail the SDKs' own 20-character minimum.

How I did it

  • src/sts.rs: role(role_arn, issuer, audiences, max_session) returns the named Role or None, replacing default_role and is_default_role. ALL_PRODUCTS is the * sentinel.
  • src/authz.rs: ceiling_permits(scopes, action).
  • src/source_api/registry.rs: the check at the top of get_bucket.
  • src/lib.rs: the key exchange resolves the named Role before its lookup, as before, and mints under it.
  • README.md: a Roles section. adrs/001, 004 and 011, in a separate commit: see below.

How to test it

  • cargo test: all suites pass. tests/sts.rs covers bare and ARN names for all three Roles, including a service-account-shaped account segment, and refusal of unknown, lower-case, pathed, suffixed and non-role names. tests/authz.rs covers that ReadOnly's sealed ceiling allows exactly the actions is_write_action calls reads, that FullAccess and _default allow every action, and that narrower scopes permit nothing. tests/keys.rs checks that a ReadOnly key's session unseals with ReadOnly's ceiling.
  • 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 against wrangler dev (wrangler 3.114) and tests/stub_api.py: 35 passed, 15 skipped. The skipped ones are the credentialed tests that need CI's GitHub token. The new test_read_only_refuses_a_write_before_anything_is_looked_up exchanges the stub's live key for ReadOnly and for FullAccess, then writes to a product the stub has never heard of. ReadOnly gets AccessDenied. FullAccess gets past the ceiling to the product lookup and gets NoSuchBucket, and so does a ReadOnly read. The upstream write itself fails closed in CI, so the product lookup is where the ceiling's path can be told apart. I also disabled the check, saw this test fail (ReadOnly got NoSuchBucket), and restored it.

Docs and ADRs

  • ADR-011 (Role-Ceiling Authorization) still describes the model. This PR implements its step 2 and its denial semantics for the two hardcoded Roles, so its status line now says it is implemented in part.
  • ADR-004 described "a single built-in Role, _default". A note under that section now points to ADR-014 and this PR, and ADR-004's "Implemented by" line lists this PR.
  • ADR-001 said assumed_role_id is "currently always _default" and that the sealed allowed_scopes is empty and "not enforced on this path". Its credential table now names the three Roles and their ceilings. Its bullet now says the registry enforces the ceiling and reads empty as no ceiling.
  • ADR-014 (docs(adr): ADR-014 service accounts; amend ADR-010 and ADR-013 #232) says "two Roles are hardcoded, FullAccess and ReadOnly, with _default kept as an alias (Add the ReadOnly Role alongside FullAccess #221)". This PR implements that, so it still holds. ADR-010's scope note is ADR-014's amendment and also holds.
  • ADR-013 as revised (docs(adr): revise ADR-013 — API keys are opaque secrets resolved by the platform #234) says a key's "role must be one the proxy serves". That still holds, and keys can now name all three Roles.
  • docs.source.coop: no page on main describes /.sts or its Roles yet. The unattended-workflow guide, Unattended workflow guide docs.source.coop#34, is where the Role names belong.

PR Checklist

  • This PR has no breaking changes. _default behaves and answers as before; only malformed ARNs that no SDK can send are newly refused.
  • I have updated or added new tests to cover the changes in this PR.
  • This PR affects the Source Cooperative Frontend & API: source.coop main's GitHub snippet names role/FullAccess, which resolves from this PR on. No source.coop change is needed.

Related Issues

Closes #221. Builds on #235, merged. ADRs: #232 (ADR-014), #234 (ADR-013 revised). Epic: source-cooperative/source.coop#491.

🤖 Generated with Claude Code

https://claude.ai/code/session_01R1eiTse4416N6uTgAy4Ddd

@claude

claude Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @alukach's task in 14s —— View job


✅ No blocking issues — safe to merge.

I read the src/ diff: sts::role, role_name, ceiling_permits, the get_bucket check and the key-exchange change. I did not run the tests or CI. The PR notes say CI won't run until #235 merges and this is retargeted.

  • Ceiling logic: ceiling_permits runs before any Source API lookup. It returns AccessDenied, the same error as other refusals. It only subtracts from the account's own permissions. Narrower scopes permit nothing.
  • Empty scopes mean no ceiling: this is safe because only the proxy mints session tokens, sealed with SESSION_TOKEN_KEY. Every existing _default session carries empty scopes.
  • ARN parsing: splitn(6, ':').nth(5)?.strip_prefix("role/") rejects arn:role/_default. A pathed resource (role/team/ReadOnly) is also refused, because the name would be team/ReadOnly and match nothing. That matches the PR description.
  • Key exchange: the Role is resolved before the key lookup, as before. The log line now names the Role.

Simplify (ponytail)

  • src/sts.rs StsCredentialRegistry: get_role clones the issuer and audiences on every call just to build a RoleConfig. That is fine at this size. If more Roles arrive, build the RoleConfigs once in new and look them up by name.

💰 Estimated review cost: $0.13 · 0m14s · 4 turns

@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

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

  • Date: 2026-09-30T03:36:51Z
  • Commit: 05434fa

alukach added a commit that referenced this pull request Sep 25, 2026
## 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>
@alukach
alukach added this pull request to stack #241 September 29, 2026 20:29
Base automatically changed from feat/opaque-api-keys to main September 30, 2026 03:34
@alukach
alukach marked this pull request as ready for review September 30, 2026 03:34
alukach and others added 2 commits September 29, 2026 20:35
`/.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
@alukach

alukach commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #235's squash merge and force-pushed: the branch is now this PR's two commits. One conflict, in adrs/004-sts.md: main gained ADR-014's amendment note (from #232) in the same place this PR adds its roles note. Both are kept, in one NOTE block, ADR-014's first. The code is unchanged and cargo test passes.

@alukach
alukach merged commit aa0abb3 into main Sep 30, 2026
20 checks passed
@alukach
alukach deleted the feat/named-roles branch September 30, 2026 03:39
alukach added a commit that referenced this pull request Sep 30, 2026
## 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>

This branch was successfully deployed

1 active deployment
preview — b4265679 Deployed Sep 30, 2026 by alukach via Deploy & Test / Deploy #391
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add the ReadOnly Role alongside FullAccess

1 participant