ci(policies): schema-compliant file-level rule_id + PolicyRunner allowlist - #17
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for an optional file-level rule_id in CI policy rule YAMLs by updating the JSON schema and relaxing PolicyRunner’s root-key validation, plus introducing a governance doc mapping file-level IDs to rule files.
Changes:
- Extend
rules.schema.jsonto allow an optional root-levelrule_id. - Update PolicyRunner to allow
rule_idas a root YAML key. - Add file-level
rule_idto existing policy YAMLs and document the mapping in governance docs.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ci/policies/schema/rules.schema.json | Adds optional root-level rule_id to the rules YAML schema. |
| tools/ci/policies/rules/versioning_svt.yaml | Adds file-level rule_id. |
| tools/ci/policies/rules/shell_safety.yaml | Adds file-level rule_id. |
| tools/ci/policies/rules/naming_snt.yaml | Adds file-level rule_id. |
| tools/ci/policies/rules/docs_drift.yaml | Adds file-level rule_id. |
| tools/ci/policies/rules/artifact_contract.yaml | Adds file-level rule_id. |
| tools/ci/checks/PolicyRunner/Program.cs | Allowlists root rule_id in YAML root key validation. |
| docs/governance/CI_POLICY.md | Adds a table mapping file-level rule_id to rule YAML files. |
Comments suppressed due to low confidence (1)
tools/ci/checks/PolicyRunner/Program.cs:954
- PolicyRunner now allowlists a root-level
rule_id, but it never validates its value. That means malformed/empty file-level IDs will silently pass PolicyRunner validation even though the JSON schema constrains the format. Consider reading the rootrule_id(if present) and applying the same regex check used forrules[*].rule_id, adding a validation error when invalid.
var unknownRootKeys = root.Children.Keys
.OfType<YamlScalarNode>()
.Select(static x => x.Value ?? string.Empty)
.Where(key => key is not ("rule_id" or "rules_schema_version" or "rules"))
.OrderBy(static x => x, StringComparer.Ordinal)
.ToList();
if (unknownRootKeys.Count > 0)
{
validationErrors.Add($"root has unknown keys: {string.Join(",", unknownRootKeys)}");
}
var versionText = GetScalar(root, "rules_schema_version");
if (!int.TryParse(versionText, out var parsedVersion) || parsedVersion != schemaVersion)
{
validationErrors.Add($"rules_schema_version must equal {schemaVersion}");
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…isfy RoC bijection
69300b8 to
cc62002
Compare
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@codex review |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tools/ci/checks/PolicyRunner/Program.cs:953
- Root-level
rule_idis now allowed, but it isn’t validated anywhere (unlikerules[i].rule_id). This means a malformed file-levelrule_idwill silently pass PolicyRunner’s YAML validation. Consider reading the optional rootrule_idand validating it against the same^CI-[A-Z0-9_-]+-[0-9]{3}$regex (and emitting a clear validation error when invalid).
var validationErrors = new List<string>();
var unknownRootKeys = root.Children.Keys
.OfType<YamlScalarNode>()
.Select(static x => x.Value ?? string.Empty)
.Where(key => key is not ("rule_id" or "rules_schema_version" or "rules"))
.OrderBy(static x => x, StringComparer.Ordinal)
.ToList();
if (unknownRootKeys.Count > 0)
{
validationErrors.Add($"root has unknown keys: {string.Join(",", unknownRootKeys)}");
}
var versionText = GetScalar(root, "rules_schema_version");
if (!int.TryParse(versionText, out var parsedVersion) || parsedVersion != schemaVersion)
{
validationErrors.Add($"rules_schema_version must equal {schemaVersion}");
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -0,0 +1,10 @@ | |||
| # CI Policy Rules | |||
| <!-- NOTE: This file should be named `CI_POLICY.MD` (uppercase extension) to be picked up by governance tooling that globs for `*.MD`. --> | |||
There was a problem hiding this comment.
The NOTE about needing to name this file CI_POLICY.MD to be picked up by governance tooling appears incorrect/misleading: tooling already scans all docs/**/*.MD files (e.g., tools/check-doc-consistency.py uses DOCS_DIR.rglob("*.MD")), and the policy ROC tooling looks specifically for filenames containing POLICY (see tools/check-policy-roc.py). Please update or remove this NOTE so it accurately reflects how the repository’s governance tooling discovers markdown files.
| <!-- NOTE: This file should be named `CI_POLICY.MD` (uppercase extension) to be picked up by governance tooling that globs for `*.MD`. --> |
|
Codex Review: Didn't find any major issues. Keep it up! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Changes:
Verification (local):