Gitignore the operator's knowledge store in both shapes bonfire init can create - #259
Merged
Conversation
…can create bonfire init seeds a .gitignore into every new user's repository. It seeded one line, for the per-machine tool scan, and nothing for the knowledge store. So an operator who enables a persistent backend and runs `git add` can stage an index built from their own private source, in a public repo published to public PyPI. The laws and the mechanism travel; the operator's contents never do. The store's single default path takes TWO on-disk shapes: the LanceDB backend makes .bonfire/vault a DIRECTORY, and the SQLite backend hands that same path to sqlite3.connect, making it a regular FILE whose vault_entries.content column holds the indexed source as cleartext. A trailing-slash pattern is directory-only and misses the file entirely, measured against real git: pattern .bonfire/vault/ + regular file -> rc=1 <NO MATCH> pattern .bonfire/vault + regular file -> rc=0 .gitignore:2:.bonfire/vault So the seeded pattern is .bonfire/vault, without the slash. It covers both shapes and, measured in the same run, over-covers nothing: sessions/, context.json and costs.jsonl stay unmatched, because operators commit their session handoffs. The narrowness pin is strengthened rather than retired, and moved into its own module because it is now one contract in both directions. It grades real git check-ignore against a real repository with the SQLite file shape materialised on disk, attributes every verdict to the seeded .gitignore so a contributor's global excludes can neither satisfy nor break it, neutralises git's own global and system config, refuses a fatal git exit instead of reading it as "not ignored", and closes the seeded set by literal equality so a third path added later cannot slip past a finite list of probe paths. One assertion was removed, with its reason: the probe for .bonfire/vault/seed.md. Nothing under src/ produces a seed.md; the string occurred in exactly one place in the whole repository, that assertion. It guarded a phantom. This is preventive, stated plainly rather than dressed up: the backend factory has no production callers today and no config key selects a backend, so nothing has leaked. The seed is what every future user inherits. The split returns the old test file BELOW its frozen size rather than raising it, and the new module is declared with a purpose rather than given a number, so no shrink-only ratchet moves. README and CHANGELOG are corrected in the same change because they now describe what init writes. Six line-anchored entries in the person-name allowlist were re-anchored to their new line numbers; their expected text is untouched. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.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
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.
CONTRACT
bonfire initseeds a.gitignoreinto every new user's repository. It seeded exactly one line — the per-machine tool scan — and nothing for the knowledge store. An operator who enables a persistent backend and runsgit addcan therefore stage an index built from their own private source, in a public repo published to public PyPI.Anta's ruling: the laws and the mechanism travel, the operator's contents never do.
WORLD — the measurement that changed the fix
The store's single default path (
src/bonfire/knowledge/__init__.py:24,vault_path=".bonfire/vault") takes two different on-disk shapes:.bonfire/vaultbecomeslancedbLanceDBBackend(vault_path=…))sqliteSqliteVaultBackend(db_path=vault_path)→sqlite3.connect(...), whosevault_entries.content TEXT NOT NULLholds the indexed source as cleartext, strictly worse to leak than embeddingsThe obvious pattern,
.bonfire/vault/with a trailing slash, is directory-only and misses the file entirely. Measured against real git, all four combinations:So the seeded pattern is
.bonfire/vault, without the slash. It covers both shapes, and in the same run it over-covers nothing:sessions/,context.jsonandcosts.jsonlstay stageable, because operators commit their session handoffs.The seed is now:
Stated plainly: this is PREVENTIVE, not a leak being closed
get_vault_backend()has zero production callers — only the factory itself, tests, and a note insrc/bonfire/engine/composition.pysaying the ingest consumer is deliberately unwired.VaultConfigcarries onlysession_dirandcontext_file, so nobonfire.tomlkey selects a backend today. Andgit ls-tree -r bonfireai/main | grep '^\.bonfire/'returns nothing — no file under.bonfire/has ever been tracked, so there is nothing to untrack and no published-history question. What this fixes is what every future user inherits.The pin is strengthened, not retired
The narrowness pin moved into its own module,
tests/unit/test_init_gitignore_width.py, because it is now one contract graded in both directions. It:git check-ignoreagainst a real repository with the SQLite file shape materialised on disk — the shape the old probe set could not see;.gitignore(.gitignore:prefix on-voutput), in both directions, so a contributor's global excludes can neither satisfy the positive half nor falsely trip the negative half;GIT_CONFIG_GLOBAL/GIT_CONFIG_SYSTEM→os.devnull) on every subprocess, so the test is machine-independent;assert returncode in (0, 1)) instead of collapsing 128 into "not ignored" — an empty result and a failure no longer look identical;{".bonfire/tools.local.toml", ".bonfire/vault"}, written as literals and never imported frominit.py(importing the guarded value is the vacuous-gate pattern). A finite list of probe paths cannot catch a third path added later; set equality can;One assertion was removed, with its reason written at the spot: the probe for
.bonfire/vault/seed.md. Nothing undersrc/produces aseed.md— the string occurred in exactly one place in the entire repository, that assertion. It guarded a phantom. Everysessions/context.json/costs.jsonlassertion is intact.FAILURE — three control rods, each watched going RED
Rod A — does the repo's own new line actually bite? Both shapes, on this tree:
Rod B — revert the seed to the trailing-slash form. The pin must go red. It does:
Rod C — silently widen the seeded set with a third narrow path no probe touches. The closed-world assertion catches what a probe list cannot:
No ratchet was moved
tests/unit/test_tools_section_is_local.py: 958 → 868, against its frozen 893. The split returns it below its number rather than raising it; the gauge will report shrink slack.tests/unit/test_init_gitignore_width.py(new, 284 lines) is declared with apurposestring rather than given a number — the shape the gauge documents for a new file, which exempts it from the package draw while still holding it to the 500-line cap.file-budget.json's diff is 3 added lines and zero deletions.cf-file-budgetexits 0 on this tree.Full suite
5778 passed, 3 skipped, 37 xfailed, 20 xpassed. Including the six external pin files that drive
_GITIGNORE_LINEand_ensure_gitignore_entryby name — the single-line primitive and the constant both keep their exact shapes for that reason, and it is documented in the code.🔴 Findings for a maintainer — not fixed here
tests/unit/test_no_persona_names_in_public_docs.py::_ALLOWLISTholds(path, lineno, full_line)tuples. Editing README and CHANGELOG shifted six of them and the pin went red — the registry pointer rotted, not the docs. I re-anchored the six line numbers and touched none of their expected text (the diff is six integers). The anchors should be content-addressed, not positional; that is the same rot this repo already retired fromexemptions.json.test_readme_quick_start_enumerates_every_created_artefactonly checks that four tokens appear in a ±600-char window, so a new artefact can never make it fire, despite its docstring claiming it pins the list. It is also now within ~90 chars of that window sliding..gitignorealready holding.bonfire/vault/gets.bonfire/vaultappended on the next init — harmless (only the new form covers the file) but the file ends with two vault lines. De-duplication would need a pattern-equivalence notioninit.pydoes not have..gitignoretakes the append branch and gets a leading blank line and no explanatory header. Pre-existing, byte-stable on re-run.src/bonfire/cli/commands/init.pyis at 499 of 500 measured lines. The next change to it must split the module, not compress comments.FILE_BUDGET_NEW 540 lines > 500forREADME.mdand744 > 500forCHANGELOG.md. Both are false: README was already 539 lines and CHANGELOG 729 atbonfireai/mainbefore this change (which adds 1 and 15 lines), neither is baselined because the shipped gauge walks.pyfiles only —file-budget.json's own notes say exactly that — andcf-file-budgetexits 0 on this tree. The pre-gate applies a Python-source size law to markdown and, because markdown is never baselined, reads every long document as a brand-new violation.Co-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com