Harden deployment waiver metadata#464
Conversation
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 428210b614
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| fn contains_authorization_value_suffix(summary: &str, scheme: &str) -> bool { | ||
| let mut words = summary.split_whitespace().rev(); |
There was a problem hiding this comment.
When a pasted auth value is followed by any explanatory text, such as summary: "rotated leaked Bearer abcdef before config apply", this helper returns false because it only inspects the last two whitespace-separated tokens. That lets the same credential-like waiver metadata this change is trying to block reach restricted posture and boot logs; scan adjacent tokens throughout the summary (while preserving the intended false-positive exclusions) instead of only checking the suffix.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c943f8b992
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if candidate_scheme.eq_ignore_ascii_case(scheme) | ||
| && is_credential_like_authorization_value(candidate_value) |
There was a problem hiding this comment.
When a waiver summary pastes a common header form without whitespace after the colon, such as Authorization:Bearer abcdef, or with punctuation attached to the scheme, such as Bearer: abcdef, split_whitespace() keeps the scheme glued to punctuation so this equality never fires and the summary is accepted. That lets the authorization value reach restricted posture and boot logs despite the new waiver metadata minimization contract; normalize colon/punctuation separators around the scheme before testing the adjacent credential value.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| if !is_valid_approval_reference(reference) { | ||
| return Err(DeploymentWaiverMetadataError::ReferenceInvalidSyntax); |
There was a problem hiding this comment.
Because reference validation only applies the approval-reference charset, a pasted value like Bearer:abcdef still passes here: it has no spaces, uses only allowed punctuation, and then gets copied into restricted posture and the new boot warning fields as the waiver reference. Treat high-confidence Bearer:/Basic: reference prefixes the same way summaries treat authorization values, or the new required metadata field remains a credential-disclosure path.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a8bc96322
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| || summary.split("-----BEGIN ").skip(1).any(|suffix| { | ||
| suffix | ||
| .split_once("-----") | ||
| .is_some_and(|(label, _)| label == "PRIVATE KEY" || label.ends_with(" PRIVATE KEY")) |
There was a problem hiding this comment.
When a summary contains an armored PGP private key marker such as -----BEGIN PGP PRIVATE KEY BLOCK-----, this check accepts it because the parsed label neither equals PRIVATE KEY nor ends with PRIVATE KEY. That lets a high-confidence private-key header pass the new waiver metadata validator and reach restricted posture or boot warnings; include PGP private-key block labels in the private-key marker rejection.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| "reference": { | ||
| "maxLength": 128, | ||
| "minLength": 1, | ||
| "pattern": "^(?!.*\\.\\.)[A-Za-z0-9._:-]+$", |
There was a problem hiding this comment.
The generated config schema still treats Bearer:abcdef or Authorization:Basic:abc123 as valid waiver references because this pattern only rejects .. and non-charset characters, while validate_deployment_waiver_metadata rejects those values at runtime. In schema-only CI/adopter tooling this accepts configs that Relay (and the matching Notary schema/source) later refuse to load, so extend the generated schema/source pattern to match the runtime credential-prefix ban and regenerate the artifacts.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
| "summary": { | ||
| "maxLength": 256, | ||
| "minLength": 1, | ||
| "type": "string" |
There was a problem hiding this comment.
The committed config schema still accepts summaries that runtime validation rejects, such as leading/trailing whitespace, control characters, Bearer abcdef, or -----BEGIN PRIVATE KEY-----, because this property only has length bounds. Schema-only CI or generators can therefore approve a waiver config that fails later at product startup; encode the same summary constraints in the generated Relay and Notary schemas/source.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9be46dcb9e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "pattern": "^(?!.*\\.\\.)[A-Za-z0-9._:-]+$" | ||
| }, | ||
| "summary": { | ||
| "type": "string", | ||
| "minLength": 1, |
There was a problem hiding this comment.
When a non-Rust producer or schema-only conformance check validates restricted registry.ops.posture.v1 output, these schema entries still accept waiver metadata that the new runtime contract rejects, such as reference: "Bearer:abcdef" or a summary containing Bearer abcdef or a private-key begin marker. That lets posture documents pass the shared public schema while carrying the credential-shaped waiver data this change is trying to keep out of restricted posture, so the posture schema should encode the same reference-prefix and summary bans in both waiver definitions.
AGENTS.md reference: AGENTS.md:L79-L81
Useful? React with 👍 / 👎.
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
Signed-off-by: Jeremi Joslin <jeremi@joslin.fr>
1 participant
What changed
BearerandBasicmaterial.Why
Free-form waiver text could carry secrets or unnecessary operational details into posture responses, logs, and audit records. The waiver itself remains useful, but its metadata needs a narrow, shared disclosure contract.
Implementer impact
Operators get a simple reference-first configuration with an optional human-readable summary. Existing non-waivable hard gates remain unchanged, and default deployments disclose less metadata.
Validation
cargo fmt --checkcargo test --locked -p registry-platform-ops -p registry-notary-core -p registry-notary-server -p registry-relaygit diff --checkEnvironment-dependent PostgreSQL and live-service tests remain ignored as designed.
Closes #84.