feat: per-job oidc_tokens block for custom audiences#1008
Open
cchristous wants to merge 12 commits intosemaphoreio:mainfrom
Open
feat: per-job oidc_tokens block for custom audiences#1008cchristous wants to merge 12 commits intosemaphoreio:mainfrom
cchristous wants to merge 12 commits intosemaphoreio:mainfrom
Conversation
Adds an optional `:audience` field to the OIDC token request handled
by `Secrethub.OpenIDConnect.JWT.generate_and_sign/1`.
Behavior:
* absent or empty list -> existing default `https://<org>.<domain>`
* single-element list -> string (RFC 7519 sec 4.1.3 convention,
required by strict-aud consumers like PyPI Trusted Publishers)
* multi-element list -> JSON array
Purely additive: existing callers that do not set `:audience` are
unaffected. Filed as part of the wider per-job `oidc_tokens` block
work; gRPC plumbing and request-struct field are out of scope for
this change and will land separately.
Introduces a per-job `oidc_tokens` map keyed by env var name, with each entry requiring an `aud` value (string or non-empty list of strings). Keys are restricted to the env var pattern via patternProperties combined with additionalProperties: false. Reserved-name semantics (`SEMAPHORE_OIDC_TOKEN`) are intentionally not enforced at the schema level and will be handled by the semantic check in a follow-up task. Adds positive and negative fixtures for the existing pipelines directory test (missing aud, invalid env var name, empty aud list).
- Rename 4 fixture files to match existing snake_case convention - maxProperties: 16 on oidc_tokens map (cap entries per job) - maxLength: 256 on aud strings, maxItems: 8 on aud arrays (hardening)
Reject SEMAPHORE_OIDC_TOKEN as a custom token name (reserved for the auto-injected default token). Env var name format ([A-Z_][A-Z0-9_]*) is enforced at the JSON schema layer via patternProperties. Duplicate token names are impossible at the yaml level (map-shaped block).
Adds a "Custom audiences" section to the OIDC user guide with a complete PyPI publishing example, and a corresponding reference section documenting the schema, behavior, and validation errors.
- Handle audience: nil (key present, nil value) by falling back to default - Add iss assertion to existing tests (locks in iss/aud independence) - Add JWTFilter interaction test (verifies aud survives filter when enabled)
Adds .fail.yml fixtures verifying maxProperties: 16, maxLength: 256, maxItems: 8, and minLength: 1 (empty aud string) on the oidc_tokens schema.
- Clarify that single-element aud lists flatten to strings (RFC 7519) - Add "coming soon" banner noting Zebra runtime injection is pending - Genericize the reserved-name validation error description - Document schema limits in the user guide - Add oidc_tokens cross-reference in the canonical pipeline-yaml reference
- Drop hard-coded "24h" TTL from user guide (rot risk; reference page intentionally omits the value) - Re-add the "when OIDC is enabled" qualifier to the user guide's default-token claim (matches reference page) - Remove leftover LLM scaffolding line in reference/openid.md - Add @SPEC to OIDCTokensValidator.validate/1
- Strengthen JWTFilter test (asserts filter actually ran by omitting
`sub` from the allowlist mock and refuting it survived into the JWT)
- Add complementary "aud not allowlisted" filter test verifying that
a user-supplied custom audience is stripped when `aud` is not in the
org claim allowlist
- Add positive boundary fixtures at exactly 16 tokens / 8 audiences /
256-char aud so a regression that tightens any of those bounds flips
a passing fixture to failing
- Mirror the "Coming soon" admonition into pipeline-yaml.md so all
three docs (user guide, openid reference, pipeline-yaml reference)
are in sync
- Add iss assertion to the single-element audience override test
(the most regression-prone unwrap path)
- Add explicit `{#oidc_tokens-block}` anchor to the openid heading so
the cross-link from pipeline-yaml.md does not depend on auto-slug
behavior
The on_exit callback called System.put_env/2 with the value returned by System.get_env/1, which is nil when the env var was unset before the test ran. System.put_env/2 has no clause for nil and crashes. Replace the bare put_env calls with a restore_env/2 helper that calls System.delete_env/1 when the saved value was nil. The test now passes regardless of host env state.
The previous "strips audience" test asserted `aud` was absent from the
final JWT after the filter ran. CI revealed that Joken's default config
injects a placeholder `aud` value ("Joken") when the claims map lacks
one, so `aud` is present after signing — just not the user's override.
The security-relevant property is "user override is suppressed", not
"aud key is absent." Updated the assertion to refute equality with the
override and clarified the test name and docstring.
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
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.
Status: Draft — proto changes pending maintainer assistance
This PR adds a per-job
oidc_tokens:block to the pipeline yaml that lets pipelines mint additional OIDC tokens with custom audiences, alongside the existing defaultSEMAPHORE_OIDC_TOKEN.Why: The OIDC
audclaim (RFC 7519 §4.1.3) identifies the token's intended consumer. Today Semaphore hardcodesaud = https://<org>.semaphoreci.com, which prevents Semaphore tokens from being used with services that strictly verify a specific audience. The motivating case is PyPI, which requiresaud="pypi"(downstream PR pypi/warehouse#19048 implementing pypi/warehouse#18882). Other ecosystems with the same constraint include npm trusted publishing, Docker Hub OIDC, and HashiCorp Vaultbound_audiences.What's in this PR
Schema (
plumber/spec/priv/v1.0.yml)Validation rules (yaml-level, fail fast):
aud. Missingaud→ schema validation failure.audis a non-empty string (≤256 chars) OR a non-empty list of non-empty strings (1–8 entries, each ≤256 chars).^[A-Z_][A-Z0-9_]*$. Up to 16 entries per job.SEMAPHORE_OIDC_TOKENrejected at semantic-validation time.Secrethub JWT generator
secrethub/lib/secrethub/open_id_connect/jwt.exlearns to honor an optional:audiencefield on the request struct. When set, it overrides the default org-URL audience. Single-element list flattens to a string per RFC 7519 convention (necessary forstrict_aud-checking consumers like PyPI). Multi-element list becomes a JSON array.req.audienceaudclaim[](default)"https://<org>.<domain>"(current behavior)["pypi"](single-element list)"pypi"(string)["a", "b"](multi-element list)["a", "b"](JSON array)Pure additive change — existing callers unaffected.
Semantic validator
plumber/ppl/lib/ppl/definition_reviser/oidc_tokens_validator.exrejectsSEMAPHORE_OIDC_TOKENas a custom token name (it's the reserved name for the auto-injected default). Wired into thedefinition_reviserwithchain alongsideJobMatrixValidator/ParallelismValidator.Documentation
User guide and reference pages updated under
docs/docs/using-semaphore/openid.mdanddocs/docs/reference/openid.md.What's NOT in this PR (needs maintainer assistance)
The full feature requires changes to
renderedtext/internal_api(the private proto-source repo) which I don't have access to:Add optional
audiencefield toGenerateOpenIDConnectTokenRequestinsecrethub.proto. Once added and.pb.exfiles regenerated,internal_grpc_api.excan pass the field through toJWT.generate_and_sign/1(which already reads it viaMap.get(req, :audience, [])).Add
OIDCTokenSpecmessage andoidc_tokensfield onJob.Specinjobs.proto(or whichever proto file currently definesSemaphore.Jobs.V1alpha.Job.Spec). Once added:plumber/pplneeds a small change to compile yamloidc_tokens(a map from name to{aud}) into the protorepeated OIDCTokenSpec oidc_tokensfield.zebra/lib/zebra/workers/job_request_factory/open_id_connect.exneeds a newload_oidc_tokens/5function that mints one additional OIDC token perJob.Spec.oidc_tokensentry (with the requestedaudience) and emits the env vars. The default-token path stays untouched.The implementation pattern for both Zebra and the plumber compilation follows the existing
secrets:block precisely.I've intentionally not modified any
.pb.exfiles in this PR (those are generated). Once the proto changes land, regenerating in the consuming services (plumber/proto,secrethub,zebra,front,guard,public-api,ee/gofer) plus the Zebra and plumber/ppl changes above complete the feature.I'm happy to do those final pieces myself if you can grant me the proto-repo access, or to review/iterate if a maintainer wants to drive them.
Tests
secrethub/test/secrethub/open_id_connect/jwt_test.exs— 7 tests:audience: nilfalls back to default (covers proto-deserialization edge case)issinvariance: overridingauddoes not changeiss(asserted on default and override paths):open_id_connect_filterenabled): assertsaudsurvives when allowlisted; complementary test assertsaudis stripped when not allowlisted (plusrefuteof a non-allowlisted claim to prove the filter actually ran).Existing 134 secrethub regression tests pass.
plumber/spec/test/pipelines/v1.0-oidc_tokens*.yml— 11 fixtures auto-discovered bySemaphoreYamlSpecTest:audforms).fail.ymlcovering each rejection rule:missing_aud,invalid_key,empty_aud_list,aud_empty_string,aud_too_long(257 chars),too_many_audiences(9 entries),too_many_tokens(17 entries)at_max_tokens= 16,at_max_audiences= 8,at_max_aud_length= 256) so a regression that tightens a bound flips a passing fixture.plumber/ppl/test/definition_reviser/oidc_tokens_validator_test.exs— 6 unit tests covering happy paths, reserved name in regular block, reserved name inafter_pipeline, and reserved name on second job in a block (proves block iteration isn't short-circuited).Schema bounds
The
oidc_tokensschema has explicit upper limits to prevent claim-bloat / DoS-style abuse:maxProperties: 16— at most 16 entries per jobmaxLength: 256— eachaudstring up to 256 charsmaxItems: 8—audlist up to 8 audiencesminLength: 1,minItems: 1— empty values rejectedEach bound has a positive fixture at the limit and a negative fixture one over.
Out-of-scope test fix
plumber/ppl/test/definition_reviser/global_job_config_test.exshad a brittleon_exitcallback that calledSystem.put_env/2with anilvalue (returned bySystem.get_env/1when the env var was unset before the test). Replaced with a smallrestore_env/2helper that callsSystem.delete_env/1for thenilcase. Strictly an improvement; the test now passes regardless of host env state. Out of scope for the feature, but it was the only thing keeping CI red on this branch.mix formatclean,mix credo --strictclean on new files.Backward compatibility
SEMAPHORE_OIDC_TOKENsemantics unchanged.:audiencefield on the JWT request map is optional (read viaMap.get(..., [])); old callers unaffected.oidc_tokensis purely additive; pipelines that don't use it behave identically to today.Open questions for maintainers
oidc_tokensto align with Semaphore's existingSEMAPHORE_OIDC_TOKENenv var vocabulary. GitLab CI usesid_tokens. Open to either.:open_id_connect. Shouldoidc_tokensride this same flag, or get a new one for staged rollout?oidc_tokensbe available in CE?secrets:is a list of objects. Map matches GitLab and is more compact for the common single-entry case. Open to flipping if you prefer.Related