Skip to content

docs(dev): design draft — cross-tool suppression audit and agent gate - #35

Merged
moneytool merged 4 commits into
mainfrom
suppression-audit-design
Oct 8, 2026
Merged

moneytool merged 4 commits into
mainfrom
suppression-audit-design

Conversation

@moneytool

@moneytool moneytool commented Oct 8, 2026 •

Copy link
Copy Markdown
Owner

Design draft for review: docs/dev/DESIGN-suppression-audit.md. Docs only, no code.

What it proposes

  • aegis audit suppressions: one inventory of the ignores in a repo across Checkov, tfsec, Trivy, tflint, KICS, Semgrep, Bandit, gitleaks, hadolint, ShellCheck, kube-linter and GitHub Actions continue-on-error. Each entry shows its age, author and reason from git blame, and blanket ignores are flagged separately.
  • An optional # aegis: reason=… owner=… expires=… annotation. Expired or unexplained suppressions fail CI in enforce mode.
  • A signed suppressions.yaml with a new suppression authority class, so an agent can't relax the rules by editing the file.
  • In PRs, only new suppressions are reported, via the Action's suppressions input, a PR comment and SARIF.
  • Hook (v1.2): ask before a coding agent writes an unannotated suppression to get CI green.

Target: v1.1, after the v1.0 budget cap (#34). Section 11 lists five open questions, each with a proposed answer.

@moneytool moneytool left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Three design-contract findings to address before merging: contextual detection, trusted enforcement policy, and consistent mode/exit behavior.

Comment thread docs/dev/DESIGN-suppression-audit.md Outdated
The hook today evaluates shell commands only. With a `suppressions.yaml` present,
`aegis install <agent>` also adds a matcher for the agent's file-edit tools (Claude Code
`Edit|Write|MultiEdit`, Codex `apply_patch`, Gemini `replace|write_file`, OpenCode `edit|write`).
The hook runs the §3 parsers on the **added lines only** and:

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P1] Parse complete before/after files rather than added lines alone. Changing only enabled = false inside an existing .tflint.hcl rule block leaves the rule identity on an unchanged line. Likewise, a new YAML list entry needs its enclosing key, and an annotation may be on an unchanged preceding line. Added-line parsing cannot reliably recognize those suppressions or their scope. Please define reconstruction of the proposed file contents, parse both complete versions with the appropriate structured parser, and compare normalized findings to detect additions and widening. Use the same semantic comparison for --since, including rule-set and scope changes and removed/changed annotations. Add fixtures for multiline configuration edits and changes that widen an existing suppression without adding a new marker.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 7f24db7, new §7. The hook and --since now rebuild the complete before and after versions of each file (for the hook, by applying the edit in memory), parse both with structured parsers, and compare normalized findings by key (tool, file, anchor) on rule set, scope and annotation. Widened is defined as: rule set grows, becomes blanket, scope grows, annotation loses a required key, or expires moves later. An edit to the annotation of an unchanged suppression is evaluated as if the suppression were new. If the edit can't be reconstructed, that has its own row in the §6 hook table. §10 lists the fixtures you asked for: tflint enabled = false, new YAML list entries, widened rule lists, blanket and scope widening, an annotation on an unchanged line, moves and renames, multi-hunk patches, and a stale old_string.


## 5. Policy: `suppressions.yaml`

Optional, in the policy directory, signed like the other files; its `principal` must hold a new

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P1] Define the trusted source of enforcement policy and prevent deletion from disabling it. Signing protects edits to suppressions.yaml, but the specified absence behavior downgrades to report-only. A PR can delete the optional file while adding suppressions unless enforcement obtains its policy elsewhere. Please specify that the enforcing PR job reads the policy and its authority/key configuration from an immutable protected base revision or deployment-owned configuration, while auditing the candidate tree as untrusted data. Candidate removal or modification of the policy must not change the effective enforcement mode. For local hooks, distinguish an unconfigured project from disappearance of a previously required enforcement policy. Include policy deletion, invalid signature, altered authority/key configuration, and report-only downgrade cases in validation.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 7f24db7, new §5.1. In CI, the policy, authority.yaml and the key configuration come from the base revision or a deployment-owned --policy-dir. The candidate tree is untrusted data: editing or deleting the policy in a PR doesn't change the effective mode and is reported as policy-changed. For local hooks, a record outside the repo remembers enforce. If the policy later goes missing or stops verifying, the project counts as broken (exit 65, and the gate escalates), not as unconfigured. Downgrading needs a validly signed report-only policy. §8 is honest that an agent running as the user can delete that record. §10 now lists the validation cases you asked for: deletion, invalid signature, replaced authority or keys, a report-only downgrade, the Action's report input against an enforce policy, and local deletion after an enforce run.

Comment thread docs/dev/DESIGN-suppression-audit.md Outdated

- `aegis audit suppressions [PATH] [--json | --sarif] [--since REF]`: the inventory. `--since`
limits it to suppressions added or widened after a git ref (the PR base).
- Exit codes, as elsewhere: 0 clean, 3 policy violations (expired, missing reason in enforce

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P2] Make report-only behavior and mode precedence explicit. The sample policy combines mode: report-only with blanket: deny, while the exit-code description says expired, denied-blanket, and over-age findings return 3 without consistently restricting that to enforce mode. The Action adds off | report | enforce, but its precedence over signed policy is undefined. Please give a behavior table: report-only emits findings without failing for policy violations; enforce returns 3 for configured violations; malformed policy has an explicit error outcome; and trusted Action configuration has a defined relationship to the policy mode. Align the hook behavior with that effective mode, including the proposed report-only behavior in question 5 and report-only treatment of continue-on-error. Replace the undefined strict annotation mode in §4 with the intended enforce/require semantics.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 7f24db7, new §6. The effective mode comes from the trusted policy; the --mode flag or Action input can only make it stricter, never looser. A table now covers report-only versus enforce for each case: violations are listed as "would fail" with exit 0 in report-only and exit 3 in enforce; unparsed files likewise; a broken trusted policy is 65 in both modes. Action report still fails against an enforce base policy, and §6 notes that protecting the workflow file is a job for rulesets. A second table gives the hook's behaviour per mode, and resolves the old Q5. §5 now says that blanket: deny yields a violation, and the mode decides whether that fails. continue-on-error is always report-only. In §4, strict is replaced by require with enforce semantics. Q5 is replaced by a question about the local enforce record.

Comment thread docs/dev/DESIGN-suppression-audit.md Outdated
YAML path, or the code line's content hash when there is no structure), plus its value: the
rule set (or `*`), scope (line < block < file < repo), and annotation.
4. **Compare** before and after by key. A finding is **new** if its key is absent before; it is
**widened** if its rule set grows, it becomes blanket, its scope grows, its annotation loses a

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

[P2] Detect newly suppressed rules when a rule set is replaced, not only when it grows. For the same normalized finding key, replacing suppressed rule A with rule B introduces a new suppression even though the set has the same cardinality and is not a superset. The current 'rule set grows' definition leaves that case ambiguous and could skip evaluation in both --since and the hook. Please define newly suppressed rules as after_rules - before_rules for explicit rule sets, and evaluate any nonempty result even if other rules were removed at the same time. Keep specific-to-blanket handling explicit, and classify a change as pure narrowing only when it introduces no newly suppressed rules and no other widening. Add an equal-size rule-replacement fixture and a mixed removal/addition fixture to §10. The original contextual parsing, trusted-policy, and mode findings are otherwise addressed by this revision.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Fixed in 60999da (§7 step 4). For explicit rule sets on both sides, the newly suppressed rules are after_rules − before_rules. Any nonempty result is evaluated, even when other rules were removed in the same change. Specific → blanket is always widened; blanket → specific suppresses nothing new. Only the newly suppressed rules are checked against policy. A change counts as pure narrowing only when it adds no newly suppressed rules and has no other widening. §10 adds the fixtures you asked for: an equal-size replacement (CKV_AWS_18 → CKV_AWS_19), a mixed removal and addition (only CKV_AWS_20 is new), and blanket → specific.

@moneytool
moneytool merged commit d6b326b into main Oct 8, 2026
9 checks passed
@moneytool
moneytool deleted the suppression-audit-design branch October 8, 2026 03:28
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