Skip to content

fix(security): reject unsupported policy fields - #313

Merged
PyRo1121 merged 1 commit into
mainfrom
fix/audit-policy-unknown-fields
Sep 8, 2026
Merged

fix(security): reject unsupported policy fields#313
PyRo1121 merged 1 commit into
mainfrom
fix/audit-policy-unknown-fields

Conversation

@PyRo1121

@PyRo1121 PyRo1121 commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Audit K1: unsupported security settings must fail explicitly

Citations

Change

Use serde deny_unknown_fields on the policy type, covering file and elevation JSON boundaries. Unknown options return a parse error, not permissive defaults. Preserve supported fields, defaults, serialization and original file contents. Remove misleading advanced examples and correct Locked-grade claims in configuration docs.

Verification

  • Red: file and elevation-payload regressions both failed because unsupported fields were accepted.
  • Green: cargo test --locked --no-default-features --lib core::security::policy::tests:: -- --test-threads=1: 13 passed.
  • cargo clippy --locked --no-default-features --lib --tests -- -D warnings: passed.
  • rustfmt on the changed Rust file and git diff --check: passed.

Intentional compatibility change: existing policy files containing unsupported keys now fail with the parser identifying the key. No file is rewritten, discarded or migrated silently. Documented remediation requires reviewing requirements before removing unsupported keys; those keys never enabled protection. No live installation/elevation or full platform matrix run. Static slop tooling abstained on the external Rust worktree; manual review and executed tests back this change.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 8, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review Completed 2026-09-08T02:44:13.172635Z d186d27 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot close or merge this PR, and I am not going to force the compatibility cutover through.

This is an explicit public/runtime break of the security-policy contract. On current main, SecurityPolicy accepts unknown TOML and JSON keys and ignores them. After this change, #[serde(deny_unknown_fields)] turns those same files and elevation payloads into PolicyError::Parse. The PR body already names that as an intentional compatibility change: existing policy files with unsupported keys stop loading.

That includes keys the documented examples previously advertised (max_cve_severity, require_sbom, verify_slsa, trusted_maintainers). Install, update, audit, and privilege-handoff all deserialize this type (load / load_optional / load_default / inherit_policy). A previously valid ~/.config/omg/policy.toml that copied those examples would start failing closed.

In-tree proof the old shape still exists: tests/common/fixtures.rs ENTERPRISE_POLICY still carries require_sbom, require_slsa, and max_cve_age_days, and tests/security_tests.rs parses that fixture through the real SecurityPolicy deserializer. The lib-only tests on this branch do not cover that path.

No merge conflict against main (06941cf9): merge-tree is clean, 0 behind / 1 ahead. There is no leftover non-breaking hole I can land independently of this fail-closed schema change. Author-scoped leftovers (changelog, updating the enterprise fixture, the still-present minimum_grade = "Locked" example) are not a reason to force the break.

I cannot close or merge from this run.

Open in Web View Automation 

Sent by Cursor Automation: Untitled

}

#[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)]
#[serde(deny_unknown_fields)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This attribute is the public contract break. File load and elevation JSON both deserialize SecurityPolicy. Keys that currently load and are ignored become parse errors. I will not land or rewrite this to force that cutover; it needs an explicit compatibility decision, not an automation merge.

@PyRo1121
PyRo1121 merged commit a5bf966 into main Sep 8, 2026
21 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant