Skip to content

feat(kb): ship the provenance tool firewall switched ON - #461

Merged
lopadova merged 1 commit into
mainfrom
feat/enable-provenance-firewall
Aug 30, 2026
Merged

lopadova merged 1 commit into
mainfrom
feat/enable-provenance-firewall

Conversation

@lopadova

Copy link
Copy Markdown
Owner

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=false restores 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

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
@lopadova lopadova added the security Security hardening and review label Aug 30, 2026
@lopadova
lopadova requested a lite review from Copilot August 30, 2026 13:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 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_FIREWALL default to true (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 lopadova added the run-e2e Fire the gated Playwright E2E job (R46 final gate) label Aug 30, 2026
@lopadova
lopadova merged commit 6537038 into main Aug 30, 2026
14 of 18 checks passed
@lopadova
lopadova deleted the feat/enable-provenance-firewall branch August 30, 2026 15:23
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

run-e2e Fire the gated Playwright E2E job (R46 final gate) security Security hardening and review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants