Audit fixes: correctness highs (topology cache-staleness + reporting scope/org leaks) - #25
Open
Krishcalin wants to merge 2 commits into
Open
Krishcalin wants to merge 2 commits into
Krishcalin wants to merge 2 commits into
Conversation
…HIGH) The graph cache fingerprint keyed the snapshot axis on max(Snapshot.created_at), but a re-collection whose config_hash matches updates the existing snapshot row in place — refreshing its NCM and version and moving updated_at, without inserting a row or touching created_at. So neither the snapshot count nor the created_at max moved, the fingerprint was unchanged, and the cache served the pre-refresh graph: a device that reconverged (or was re-parsed by a newer parser) kept answering path/segmentation queries from stale routes — a false unreachable, or a stale allowed. Key it on max(Snapshot.updated_at), which moves on both an insert and an in-place refresh. Adds test_an_in_place_snapshot_refresh_invalidates_it. 12 topology-cache tests pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three reporting paths ignored the caller's object-level scope and/or the org, disclosing out-of-scope data — worse than a live view because a report is a frozen, hash-signed, downloadable artefact that keeps disclosing after the scope is corrected. - Executive summary (HIGH): `devices_total` was `count(Device)` with no scope and no org filter, while the finding counts were scoped — so a caller entitled to 10 devices saw a total of every device in every org, and `devices_without_findings` mixed a global total with a scoped numerator (500 - 2 = 498). Now scoped and org-filtered like the findings. - Exceptions register (HIGH): queried every FindingException with no filter at all, handing a group-scoped auditor every accepted risk in the database — device, free-text justification and approver. Now filtered to org, and for a scoped caller to exceptions on a visible device or a device group in scope. - _require_group (LOW): fetched a DeviceGroup with no org filter, echoing another org's group name back through the "contains no devices you can see" error. Now org-filtered. Adds TestReportsRespectCallerScope: a group-scoped exec summary counts only in-scope devices, and the register lists only in-scope exceptions (no out-of-scope justification leaks). 73 report tests pass. The trend report's cross-scope comparison (MEDIUM) is a related but distinct fix, deferred to the mediums batch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This was referenced Sep 30, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Audit fixes — correctness highs: cache-staleness + reporting scope leaks
From the 2026-09-30 adversarial audit (
docs/audit/2026-09-30-adversarial-audit.md). Basemain; independent of #22/#23/#24.Topology cache serves a stale graph (
de49d17, HIGH)The graph cache fingerprint keyed its snapshot axis on
max(Snapshot.created_at). A re-collection whoseconfig_hashmatches updates the existing snapshot row in place — refreshing its NCM/version and movingupdated_at, without inserting a row or touchingcreated_at. So the fingerprint didn't move and the cache served the pre-refresh graph: a device that reconverged (or was re-parsed) kept answering path/segmentation queries from stale routes — a false unreachable, or a stale allowed. Fixed by keying onmax(Snapshot.updated_at), which moves on both insert and in-place refresh. + regression test.Reporting scope/org leaks (
c9543c8, HIGH ×2 + LOW)Reports are frozen, hash-signed, downloadable artefacts, so a scope leak keeps disclosing after the scope is corrected.
devices_totalwas a globalcount(Device)— no scope, no org — while findings were scoped, so a caller entitled to 10 devices saw the whole estate's count and a fictionaldevices_without_findings. Now scoped + org-filtered.FindingExceptionwith no filter, handing a group-scoped auditor every accepted risk in the DB (device, justification, approver). Now org-filtered, and for a scoped caller limited to exceptions on a visible device or a group in scope._require_group(LOW): fetched a group with no org filter, echoing another org's group name via the error path. Now org-filtered.TestReportsRespectCallerScope.Deferred
The trend report's cross-scope comparison (MEDIUM) — a related but distinct fix — goes in the mediums batch.
Tests
Green across the touched suites: topology-cache (12), reports (73).
🤖 Generated with Claude Code