v0.47.0: tenant isolation across the evidence path#179
Merged
Conversation
The reference HTTP server's GET /v1/audit/actions/{id}/chain read
state.audit._by_action with no tenant check, returning any action's full
chain (agent_id, tool_name, payload) to any caller who knew the action_id,
despite the registry advertising per-tenant isolation. Add a tenant-scoped
read (AuditTrail.get_action_chain_scoped) gated on X-Vaara-Tenant; unknown
and cross-tenant actions both 404 with the same body so the response is not
an existence oracle. The scoped read also resolves chain positions in one
pass, removing the O(n^2) _records.index per event.
SSE notification broadcast (_mcp_notify HttpRouter.deliver) ignored the
per-session tenant: two tenants on a shared upstream each received the
other's upstream-pushed notifications. Progress notifications now scope the
broadcast to the captured originating tenant (threaded from the inflight
map); unattributable log notifications (no progressToken) suppress fan-out
across distinct tenant scopes on a shared upstream and broadcast only within
a single scope, preserving single-tenant and empty-tenant behavior.
Adds negative isolation tests for both paths. 255 server/proxy/notify tests
pass; ruff clean.
Completes the tenant-isolation pass on the evidence path. The reference server's audit-chain read and SSE broadcast were already scoped per tenant in the previous commit; this binds tenant identity into the tamper-evident hash chain itself, so the multi-tenant evidence claim holds against a signed anchor rather than only in the live store. Security: - AuditRecord gains a chain_version field. Records appended from this release on (chain v2) fold tenant_id and chain_version into compute_hash, so re-attributing a record to another tenant after the fact breaks verify_chain() instead of passing silently. Binding chain_version too means a downgrade to v1 (which would strip the tenant binding) also breaks the chain. - Pre-v0.47 records carry chain_version 1 and hash exactly as before (tenant_id excluded), so existing trails and signed exports re-verify byte for byte. A fixed-digest test guards that v1 hash against future drift. - The standalone verifier (vaara.audit.verify) mirrors the v1/v2 rule, kept in lockstep with AuditRecord.compute_hash. Storage: - SQLite schema v4 adds a chain_version column with a migration defaulting legacy rows to 1. write_record persists the record's chain_version and, for v2 records, stores tenant_id verbatim (the instance-scope substitution is confined to v1, where tenant_id was never hashed) so stored == hashed and the chain re-verifies on reload. Tests: - v1/v2 hash behavior, fixed v1 digest, live-trail re-attribution detection, SQLite v2 round-trip, signed-export round-trip through the standalone verifier, and a v3->v4 migration that proves a legacy record with a populated tenant_id column still verifies under v1 rules. 1087 passed / 12 skipped, ruff clean.
📝 WalkthroughWalkthroughThis PR hardens multi-tenant security by versioning the audit hash-chain, binding tenant identity into tamper-evident hashes for new records, scoping HTTP audit-chain reads by tenant, filtering SSE broadcasts per tenant, and migrating the SQLite schema from v3 to v4. ChangesMulti-Tenant Audit Hardening
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| try: | ||
| reloaded = backend.load_trail(strict=True) # raises if chain broke | ||
| finally: | ||
| backend.close() |
| ) | ||
| ) | ||
| finally: | ||
| backend.close() |
| try: | ||
| reloaded = reopened.load_trail(strict=True) # raises if chain broken | ||
| finally: | ||
| reopened.close() |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_sqlite_backend.py (1)
296-300: 💤 Low valueOptional: use a
withblock for the backend.CodeQL flags the manual
try/finallyclose. SinceSQLiteAuditBackendis a context manager and the rest of this file already useswith, this aligns for consistency. The in-memoryreloadedtrail remains usable after the block.♻️ Proposed refactor
- backend = SQLiteAuditBackend(db_path) # migrates 3 -> 4 - try: - reloaded = backend.load_trail(strict=True) # raises if chain broke - finally: - backend.close() + with SQLiteAuditBackend(db_path) as backend: # migrates 3 -> 4 + reloaded = backend.load_trail(strict=True) # raises if chain broke self._assert_current(db_path)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_sqlite_backend.py` around lines 296 - 300, Replace the manual try/finally close with a context manager: use a with block when instantiating SQLiteAuditBackend so that backend.close() is called automatically; call backend.load_trail(strict=True) inside the with and assign the returned reloaded trail to a variable (it remains usable after the with). Update the code around SQLiteAuditBackend and load_trail to follow the pattern used elsewhere in the file.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/test_sqlite_backend.py`:
- Around line 296-300: Replace the manual try/finally close with a context
manager: use a with block when instantiating SQLiteAuditBackend so that
backend.close() is called automatically; call backend.load_trail(strict=True)
inside the with and assign the returned reloaded trail to a variable (it remains
usable after the with). Update the code around SQLiteAuditBackend and load_trail
to follow the pattern used elsewhere in the file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a8649c1-2fc9-4b77-8151-d02d5fa6c0be
📒 Files selected for processing (17)
CHANGELOG.mdclients/ts/package.jsonpyproject.tomlserver-vaara-server.jsonserver.jsonsrc/vaara/__init__.pysrc/vaara/audit/sqlite_backend.pysrc/vaara/audit/trail.pysrc/vaara/audit/verify.pysrc/vaara/integrations/_mcp_notify.pysrc/vaara/integrations/mcp_proxy.pysrc/vaara/server/routes.pytests/test_integrations_mcp_proxy.pytests/test_mcp_notify.pytests/test_server.pytests/test_sqlite_backend.pytests/test_v040_tenant.py
vaaraio
added a commit
that referenced
this pull request
May 31, 2026
* feat(audit): external time anchor for the hash chain (v0.48.0) Adds vaara.audit.timeanchor: AuditTrail.anchor_head() obtains an RFC 3161 trusted timestamp over the current chain head (a SHA-256 digest) from an external Time-Stamp Authority, so the chain's existence is provable against a clock outside Vaara's trust boundary even if the signing key is later compromised. RFC 3161 underpins eIDAS qualified electronic timestamps, which makes this regulator-grade evidence under EU AI Act Article 12. Token verification is offline (verify_anchor, verify_anchor_over_records) and binds the anchor to a specific record, so a rewritten chain or a token over a different digest is rejected. HTTP uses the stdlib; the ASN.1 and signature checks need the new optional 'timeanchor' extra (asn1crypto + cryptography). This is the anti-backdating mechanism cited by the new server-side signed execution-record SEP draft (docs/sep/sep-server-execution-record.md), the follow-up to SEP-2817 that Vaara is positioned to author. Also reframes the public lead back to EU AI Act runtime evidence and data sovereignty (README, package descriptions, MCP manifests, vaara.io) with the tamper-evident receipt as the mechanism, not the headline. 1096 passed / 12 skipped, ruff clean, mypy clean on the new module. * test(audit): use context-manager form for SQLiteAuditBackend in new tests Clears three CodeQL 'should use a with statement' advisories on the v0.47 audit tests (test_sqlite_backend.py, test_v040_tenant.py). The fix was authored during PR #179 but its commit never reached the remote before that PR squash-merged, so the advisories are still live on main; this lands it. No behavior change. --------- Co-authored-by: vaaraio <267591518+vaaraio@users.noreply.github.com>
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
2 participants
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.
v0.47.0: tenant isolation across the evidence path
Closes the tenant-isolation findings from the v0.46.0 audit. Two of the three
were verified by reproduction against live code before fixing; the third (the
registry "lag") was a stale read and is not a code issue.
What was real, and how it is closed
Cross-tenant audit-chain read (was HIGH).
GET /v1/audit/actions/{id}/chainread the in-memory action map with no tenant check, so any caller who knew an
action_idcould read another tenant's full chain. Reproduced: a tenant-Bcaller read tenant-A's tool parameters. Now served through a tenant-scoped read
gated on
X-Vaara-Tenant; unknown and cross-tenant actions both return 404 withan identical body, so the response is not an existence oracle.
SSE broadcast leak (was MEDIUM). Upstream-pushed notifications on a shared
upstream fanned out to every tenant. Now scoped to the originating tenant;
unattributable log notifications broadcast only within a single scope.
Tenant identity outside the hash chain (was MEDIUM). Reproduced: mutating a
record's
tenant_idfrom A to B leftverify_chain()green. NowAuditRecordcarries a
chain_version; v2 records foldtenant_id(and the version itself,to block a downgrade) into the hash, so re-attribution breaks the chain. After
the fix the same mutation is detected.
Backward compatibility
Pre-v0.47 records carry
chain_version1 and hash exactly as before, so existingtrails and signed exports re-verify byte for byte. A fixed-digest test guards the
v1 hash against drift. SQLite gains a
chain_versioncolumn (schema v4) with amigration defaulting legacy rows to 1; a migration test proves a legacy record
with a populated
tenant_idcolumn still verifies under v1 rules. The standaloneverifier mirrors the v1/v2 rule.
Verification
1087 passed / 12 skipped, ruff clean. New tests cover v1/v2 hash behavior, the
fixed v1 digest, live re-attribution detection, SQLite round-trip, signed-export
round-trip through the standalone verifier, and the v3 to v4 migration.
Summary by CodeRabbit
Security Enhancements
Chores