Model secrets as baseline plus per-mechanism additions - #238
Merged
Conversation
Add a baseline set - the App-token secrets the fleet-wide App-signed merge-bot needs on every repo (forbids CODEGEN_APP_ID) - so they are implicit, not repeated per-repo or tied only to codegen. nuget-oidc now requires NUGET_USERNAME (the NuGet/login profile name); add a codecov mechanism (CODECOV_TOKEN) and a featureMechanisms map. Relax validate.py: an OIDC mechanism may require a non-secret stored value (username); the forbids list, not an empty requires, enforces "no static key". Normalize registry requiredSecrets (Utilities/LanguageTags -> NUGET_USERNAME; drop the now-baseline App secrets) and the codegen type's requiredSecrets. Update the schema (baseline required, featureMechanisms, shared secretSet def). Validated with jq + spec/validate.py. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the secrets specification to separate fleet-wide baseline secrets from per-target/per-feature additions, aligning the registry and validation logic with the audit findings about OIDC mechanisms and non-secret stored values.
Changes:
- Introduces a required
baselinesecret set inspec/secrets.jsonand updates the schema to support sharedsecretSetdefinitions plusfeatureMechanisms. - Updates
nuget-oidcto requireNUGET_USERNAME, adds acodecovmechanism, and normalizes how registries list only domain-specific additions. - Adjusts
spec/validate.pyto stop rejecting OIDC mechanisms solely because they have non-emptyrequires.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/validate.py | Relaxes OIDC validation around requires to allow non-secret stored values. |
| spec/secrets.schema.json | Adds baseline and $defs.secretSet, plus optional featureMechanisms. |
| spec/secrets.json | Restructures secrets into baseline + per-mechanism sets; adds Codecov and NuGet username requirement. |
| spec/project-types.json | Removes codegen’s per-type required secrets now that baseline applies fleet-wide. |
| registry/repos.json | Updates NuGet repos to list NUGET_USERNAME as the required domain-specific addition. |
Add the correct OIDC guarantee to validate.py: an oidc-labeled publish entry must have a non-empty forbids (it must block its static-credential counterpart), replacing the removed empty-requires proxy. Use the action slug actions/create-github-app-token in baseline.workflowNeeds (consistent with the other mechanisms' slugs); move the client-id-not-app-id detail to the note. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
Add a (oidc | static-secret) to each publish mechanism and check the repo's publish label against it directly, instead of inferring from requires-emptiness (which broke once OIDC mechanisms could require NUGET_USERNAME) - so a nuget target mislabeled static-secret is now caught. Keep the spec-integrity check that an oidc-kind mechanism forbids a static credential. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
validate.py now treats baseline as a required top-level key and reads forbids via get() so a malformed mechanism reports an error instead of crashing. codecov gains an explicit static-secret kind, and the schema requires kind for every entry under mechanisms (a new mechanism def = secretSet plus required kind) while keeping it optional for baseline. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
…bids The validator asserts every mechanism has a recognized kind (oidc or static-secret) before the kind-based label check relies on it, and reads requires/forbids via get() so a malformed secrets.json reports a clear error instead of raising KeyError (CI runs jq empty plus this script, no JSON-schema validation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
One structural check verifies baseline and each mechanism are objects with list requires/forbids, a recognized kind, and the oidc-forbids integrity, returning a clear error before the cross-reference loops. Since CI runs no JSON-schema validation, a malformed secrets.json now reports a message instead of raising AttributeError/TypeError. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
…anisms A shared helper now validates baseline and every mechanism have array requires/forbids (and mechanisms a valid kind), and targetMechanisms is an object mapping to a string or null, so any malformed secrets.json reports a clear error before the cross-reference loops. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
…n comment check_secret_set now requires requires/forbids to be arrays of strings (a non-string element would break set membership downstream), and the shape check validates the optional featureMechanisms map like targetMechanisms. Shorten the block comment to the load-bearing why (CI runs no JSON-schema validation). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Pieter Viljoen <ptr727@users.noreply.github.com>
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.
Restructures
spec/secrets.jsoninto generic baseline + domain-specific per-mechanism additions (directive 4), fixing the secret-model gaps the audits surfaced.CODEGEN_APP_*) the fleet-wide App-signed merge-bot needs on every repo; implicit, no longer repeated per-repo or tied only to codegen. ForbidsCODEGEN_APP_ID.NUGET_USERNAME(theNuGet/loginprofile name); codecov mechanism added (CODECOV_TOKEN);featureMechanismsmap for non-publish features.forbidslist - not an emptyrequires- is what enforces "no static key". (The old check hard-failed any OIDC mechanism with a non-emptyrequires.)registryrequiredSecrets(Utilities/LanguageTags ->NUGET_USERNAME; dropped the now-baseline App secrets) + the codegen type; schema updated (baseline required,featureMechanisms, sharedsecretSetdef).Validated locally:
jq empty+python3 spec/validate.py-> 8 cataloged / 14 backlog classify cleanly.🤖 Generated with Claude Code