Skip to content

Nothing reserves the synthetic rule names, so an audit record cannot prove what decided it #84

Description

@plusky

Policy::validate (crates/bugwarden-core/src/policy.rs) performs no check on Rule::name at all — no reserved words, no uniqueness, not even non-empty. Rule::name is a plain deserialized String.

So an operator can write:

[[rule]]
name = "default"

and that rule's decisions become indistinguishable in the audit stream from decisions the policy default made. The same holds for unavailable, min_bug_age_days, and the <name>:unreadable-metadata suffix form. Two rules may also share one name.

Verified empirically against the real Policy::from_toml_str: a policy declaring rules named default, unavailable, min_bug_age_days and x:unreadable-metadata parses, validates, and goes on to classify under those names. Duplicate names are accepted too.

Why it matters

guard.rule is the only field saying what decided a per-bug assessment, and the audit stream is the only place that fact surfaces at all (I3 keeps it from the client). A log consumer counting default-decided calls by rule == "default" over-counts silently, and an incident reviewer cannot tell a named rule's grant from the policy default's.

It is recoverable — every record carries policy_hash, so a reviewer holding the digest-matching policy can grep it for a rule named default — but that is a manual step nothing signals is necessary.

Scope: footgun, not a guard weakness

No invariant is breached. Rule names come only from the startup TOML, immutable at runtime and unreachable from MCP (I1), so there is no client-side attack. The malicious-operator branch collapses on threat model: an operator who wants to disguise a grant already controls the whole policy and the audit configuration. This is an operator footgun and a log-consumer correctness problem, not an access-control one.

PR #83 documents the collision (GuardInfo.rule rustdoc, Access::Denied rustdoc, DESIGN.md, README) rather than fixing it, because rejecting the names is a behaviour change and #83 was documentation-only.

Options

  1. Reject the reserved spellings in Policy::validate — a startup error, so a behaviour change, but the same error class as the dead-configuration checks validate already performs (operations = [], a capability list on an allow/deny rule, a create-scoped restrict rule whose capabilities disagree with its scope). Would make guard.rule injective for a consumer with no policy file in hand. Also worth deciding whether duplicate rule names should be rejected while there.
  2. Namespace the synthetics in the record — e.g. a separate rule_kind field, or a prefix. A schema change, so Adopt the 2026-07-28 MCP revision — audit schema v1 assumes three things it removes #34/v2 territory.

Option 1 is the smaller change and does not touch the record format.

Acceptance criteria

  • A decision recorded on whether the names are reserved, namespaced, or deliberately left colliding.
  • If validation rejects them, the error names the offending rule and the reserved spelling, and the :unreadable-metadata suffix form is covered too.
  • A test pins that a policy naming a rule default is rejected (or, if the decision goes the other way, that the collision is tolerated deliberately).
  • Whether duplicate rule names are also rejected is decided in the same pass.

Surfaced during the adversarial review of #83 (issue #67), and independently confirmed by a second reviewer.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions