Repository navigation
feat(kb): ship the provenance tool firewall switched ON - #461
Merged
Merged
Conversation
ADR 0028 specified default OFF for this phase and the product owner has reversed it. The reversal is recorded in the ADR itself, not left as a discrepancy between a document and a config file - a default that contradicts its own ADR is the drift that file exists to prevent. The ADR's caution stands and upgraders should know it: a deployment already ingesting email will find that turns grounded in a message stop being able to act. What decided it the other way is that a security control shipping OFF protects nobody until somebody remembers to switch it on, and the chain it closes is not hypothetical - IMAP ingests what anyone can send, ingested content becomes grounding, and the same platform exposes tools to the model, with no boundary between them. The failure direction is the safe one: the turn declines to act rather than acting on a stranger's instructions, and the answer still comes back from the same context with the same citations. Setting KB_PROVENANCE_TOOL_FIREWALL=false restores the previous behaviour exactly. Six places move together - config default, the service's fallback, .env.example, the ADR, the doc-site page and the README - because a default documented in five places and changed in one is how this kind of thing goes wrong. The test that asserted the shipped default now asserts ON rather than being deleted: the default IS the decision, so it is worth a test of its own. Both states stay covered (R43). 1337 tests green across the KB, API and MCP suites. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QPJodaHYMJ3wppek7aYTBX
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The default flip is applied consistently across runtime config, service fallback, tests, and docs, with both enabled/disabled states still explicitly covered.
Pull request overview
This PR flips the shipped default for the ADR 0028 phase-3 provenance tool firewall from OFF to ON, and updates the code + tests + operator-facing docs so the default is consistent and explicitly asserted.
Changes:
- Switched
KB_PROVENANCE_TOOL_FIREWALLdefault totrue(config + service fallback). - Updated the default-assertion test to expect “ships ON”, while still covering the OFF path explicitly.
- Updated documentation surfaces (ADR, README, docs-site page,
.env.example) to match the new default and the upgrade impact.
File summaries
| File | Description |
|---|---|
config/kb.php |
Changes the env default for the firewall to true and updates rationale text. |
app/Services/Kb/Provenance/ProvenanceToolFirewall.php |
Aligns config(..., <fallback>) fallback to default-enabled behavior. |
tests/Feature/Kb/ProvenanceToolFirewallTest.php |
Updates the shipped-default assertion to ON; keeps explicit OFF-path coverage. |
.env.example |
Sets KB_PROVENANCE_TOOL_FIREWALL=true and updates operator guidance comments. |
docs/adr/0028-source-acl-mirroring-and-ingest-provenance.md |
Records the decision reversal inside the ADR to prevent doc/config drift. |
docs-site/tool-firewall.mdx |
Updates the public doc-site page configuration snippet and narrative to default ON. |
README.md |
Updates the security/compliance feature row to state default ON and the upgrade implications. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
lopadova
added a commit
that referenced
this pull request
Sep 1, 2026
lopadova
added a commit
that referenced
this pull request
Sep 1, 2026
…#470) The roadmap table jumped from v8.26.0 straight to v8.32→v8.34, and the changelog's newest entry was v8.30.0. Five versions had shipped without a roadmap row and five without a changelog entry, so the two tables that answer "what is in this product" disagreed with each other and with the code. Backfilled from verifiable sources rather than reconstructed from memory: merge dates come from the merge commits (#397 2026-07-10, #402 2026-08-24, #403/#404 2026-08-06, #424 2026-08-30, #443/#448/#452 2026-08-29, #455/#461 2026-08-30), and the substance from the ADRs that accompanied each cycle plus the CLAUDE.md sections written when the work landed. Where a changelog entry already existed (v8.27, v8.28, v8.30) the roadmap row summarises that entry rather than re-deriving the history. Roadmap gains v8.27.0, v8.28.0, v8.29.0, v8.30.0, v8.31.0 and v8.35.0, so the table now runs unbroken from v8.25 to the present. Changelog gains v8.29.0, v8.31.0, v8.32.0, v8.33.0, v8.34.0 and v8.35.0. v8.27.0 also stops claiming to be "in progress": PR #424 merged on 2026-08-30 and completed it, so the heading was three weeks stale and would have told a prospective consumer the API connector was unfinished. v8.35.0 is this cycle — laravel/ai 0.8→0.11 and laravel-flow 1.1→2.5 landing together, the flow surface closing its R32/R44/R45 gaps, and the empty-tenant fix that came out of review. The entry keeps the parts that are load-bearing for anyone upgrading: that the tenant on converted history was the real risk and why a `default` stamp there would have been undetectable, that four of the v2 breakages fail at boot rather than gradually, and that the first R32 test was theatre because routes register at boot from a flag the test flipped too late. R45 parity verified rather than assumed: every version documented here already has its deep doc-site page — connectors-api, team-management, case-study-email-datasets, system-administration, source-permissions, ingest-provenance, tool-firewall and flow-orchestration all exist. The doc-site was never what fell behind; the README was.
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
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.
ADR 0028 specified default OFF for this phase. The product owner has reversed that, and the reversal is recorded in the ADR itself rather than left as a discrepancy between a document and a config file — a default that contradicts its own ADR is precisely the drift that file exists to prevent.
The trade, stated plainly
The ADR's caution stands, and anyone upgrading should know it: a deployment already ingesting email will find that turns grounded in a message stop being able to act.
What decided it the other way is that a security control shipping off protects nobody until somebody remembers to switch it on — and the chain it closes is not hypothetical. It is three ordinary facts about this product with no boundary between them: IMAP ingests what anyone who can send an email writes, ingested content becomes retrieval grounding, and the same platform exposes MCP tools to the model.
The failure direction is the safe one. The turn declines to act rather than acting on a stranger's instructions, and the answer still comes back from the same context with the same citations — only the tool loop is withheld.
KB_PROVENANCE_TOOL_FIREWALL=falserestores the previous behaviour exactly.Six places, moved together
Config default · the service's own fallback ·
.env.example· the ADR · the doc-site page · the README.That is the point of the diff being this wide for a one-word change: a default documented in five places and changed in one is how a codebase ends up telling operators something that stopped being true.
Tests
The test that asserted the shipped default now asserts ON rather than being deleted — the default is the decision, so it deserves a test of its own. Both states stay covered (R43): the enabled path in every behavioural test, the disabled path explicitly.
1337 tests green across the KB, API and MCP suites, confirming the flip does not disturb the chat path where no document carries an external provenance tier — which is every corpus that has not configured a connector declaring one.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QPJodaHYMJ3wppek7aYTBX