tidaldb/.sdlc/features/m10-community-policy-engine/audit.md

3.6 KiB
Raw Blame History

Audit: Community Policy Engine (m10-community-policy-engine)

Security

  • Write enforcement is fail-closed. CommunityPolicyEvaluator::check_write returns Err on any violation; there is no silent pass-through. An unknown role returns an error (not a default allow).
  • Policy is schema-sealed. CommunityPolicy rules are validated at SchemaBuilder::build() time — signal names are cross-referenced against the schema. A policy referencing a non-existent signal is rejected before the schema is usable, preventing misconfiguration at runtime.
  • Allow/deny conflicts are caught at build time. A signal in both allowed_write_signals and denied_write_signals fails schema construction. No ambiguous runtime behavior.
  • Read suppression is additive and conservative. The suppressed signal set is a union of denied_read_signals plus any signal not in the (non-empty) allowed_read_signals list. A signal not explicitly allowed when the allow list is non-empty is suppressed — this is the correct conservative interpretation.
  • No privilege escalation path. Community context is caller-supplied but role lookup is schema-bound. A caller cannot invent a role that grants extra access — an unrecognized role is an error, not a fallback-to-admin.

Correctness

  • Spec coverage: all 10 test matrix scenarios (I1I10) are implemented and passing.
  • Empty allow/deny lists correctly mean "no restriction" (admin role). A role with no lists set passes all writes and suppresses no reads.
  • Signal suppression does not affect candidate generation. The suppressed set only influences ProfileExecutor scoring — items with suppressed signals remain in the candidate set. I5 verifies result count is unchanged.
  • Decay-rate index is used correctly. read_decay_score(entity, signal, 0) in tests uses index 0 (the only configured lambda) — not a timestamp. Prior bug (using ts.as_nanos() as the index) was fixed.
  • Universe bitmap registration required for retrieve. Tests I5 and I6 correctly call write_item_with_metadata before retrieve queries to register items in the universe bitmap — the scan strategy requires this.

Performance

  • is_suppressed is O(1) with fast-path early exit. The check !self.suppressed_signals.is_empty() short-circuits before the HashSet::contains lookup when no community context is active. No cost on unconstrained queries.
  • Suppressed set construction is O(n) in schema signals. Built once per query in stage3_score; schema signal count is bounded and small. Not a hot path concern.
  • No lock contention. ProfileExecutor is constructed per-query and owns its suppressed_signals set. No shared mutable state.

Observability

  • Error messages reference the specific signal name or role name that caused the violation (e.g. "policy violation: signal 'pin' is denied for role 'member'"). Callers can act on the error without guessing.
  • Schema validation errors name the duplicate, unknown, or conflicting signal/role for immediate diagnostics.

Tech Debt

  • Module duplication resolved. The stale entities/revocation.rs flat file (left over from the previous session's linter pass) was removed. The canonical implementation is entities/revocation/mod.rs + entities/revocation/tests.rs. No functional impact, but the conflict would have prevented compilation until fixed.
  • ProfileExecutor::new changed from const fn to fn to accommodate HashSet::new(). This is the correct approach; const fn was not required by any caller.

Verdict

Approved. No security issues, no correctness gaps, no performance concerns. Implementation is complete and production-ready.