From 5a36189256f600b754844ee2e223265aa3a12b59 Mon Sep 17 00:00:00 2001 From: Antawari Date: Tue, 28 Jul 2026 21:57:01 -0600 Subject: [PATCH] Gitignore the operator's knowledge store in both shapes bonfire init 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 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) --- .gitignore | 14 + CHANGELOG.md | 15 + README.md | 5 +- file-budget.json | 3 + src/bonfire/cli/commands/init.py | 123 +++++--- tests/unit/test_init_gitignore_width.py | 284 ++++++++++++++++++ .../test_no_persona_names_in_public_docs.py | 12 +- tests/unit/test_tools_section_is_local.py | 207 ++++++------- 8 files changed, 500 insertions(+), 163 deletions(-) create mode 100644 tests/unit/test_init_gitignore_width.py diff --git a/.gitignore b/.gitignore index ae0ac104..069e6b8d 100644 --- a/.gitignore +++ b/.gitignore @@ -216,3 +216,17 @@ __marimo__/ # per-run copy of the operator's OAuth credential plus claude-stream logs and a # clone of the fixture target — local-only, must never be committed. .e2e-runs/ + +# Bonfire's own knowledge-backend store. .bonfire/vault is the documented +# default path for BOTH on-disk shapes that store can take, and one pattern +# has to cover both: the LanceDB backend makes it a DIRECTORY holding a vector +# index built from this repo's own source, while the SQLite backend hands the +# same path to sqlite3.connect and gets a REGULAR FILE whose +# vault_entries.content column holds that source as cleartext. Hence NO +# trailing slash — a directory-only pattern cannot reach the file shape, which +# is the worse of the two to publish. The laws and the mechanism travel; the +# operator's contents never do. No other .bonfire-specific pattern lives here: +# sessions/ handoffs, context.json and costs.jsonl stay committable, which is +# why there is no bare .bonfire/ line. (The broad patterns elsewhere in this +# file — *.log, .env*, .cache — reach into .bonfire/ as they reach everywhere.) +.bonfire/vault diff --git a/CHANGELOG.md b/CHANGELOG.md index eabb6f42..2f3e8751 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -76,6 +76,21 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Changed +- `bonfire init` now seeds two `.gitignore` entries instead of one: + `.bonfire/tools.local.toml`, the per-machine tool inventory, and + `.bonfire/vault`, where the knowledge backend keeps its store once an + operator enables a persistent one. The store entry carries no trailing + slash because `.bonfire/vault` is the documented default path for both + on-disk shapes that store can take, and one pattern has to cover both: + the LanceDB backend makes it a directory holding a vector index built + from the operator's own source, while the SQLite backend hands the same + path to `sqlite3.connect` and gets a regular file whose row content + holds that source as cleartext. A directory-only pattern would leave + the file shape stageable, which is the worse of the two to publish. + Nothing else under `.bonfire/` is covered: `sessions/` handoffs, + `context.json` and `costs.jsonl` stay committable. Re-running `init` + still duplicates neither line, and the success output names both + entries. The repo's own `.gitignore` moved to the same pattern. - README: the "What's Not There Yet" list is re-derived against the current tree. `bonfire run` exists and is documented; `status`, `resume` and `handoff` are implementations rather than stubs; four diff --git a/README.md b/README.md index 8698394f..f2ae2f26 100644 --- a/README.md +++ b/README.md @@ -117,8 +117,9 @@ Claude Code session. # - .bonfire/ (per-project state directory) # - agents/ (role-local prompt + identity-block overrides; see # Extension Points) -# - .gitignore entry: `.bonfire/tools.local.toml` (appended if -# missing; idempotent — re-running does not duplicate the line) +# - .gitignore carrying two entries, appended if missing (idempotent; +# re-running duplicates neither line): `.bonfire/tools.local.toml` +# and `.bonfire/vault`, the knowledge-backend store bonfire init . # Drive a prompt through a workflow and the pipeline engine. Needs a diff --git a/file-budget.json b/file-budget.json index a0e9ed4a..222fe930 100644 --- a/file-budget.json +++ b/file-budget.json @@ -25,6 +25,9 @@ "tests/unit/test_events.py": 675, "tests/unit/test_git.py": 754, "tests/unit/test_github.py": 846, + "tests/unit/test_init_gitignore_width.py": { + "purpose": "the two-direction width contract for the .gitignore that bonfire init seeds: the operator-local paths under .bonfire/ are covered in BOTH on-disk shapes the knowledge store's single default path can take (a LanceDB directory and a SQLite regular file), while every committable sub-path (sessions, context.json, the cost ledger) stays stageable. Split out of test_tools_section_is_local.py, which owns the tools-local-file pins and was at its frozen size: this is one contract with its own module, and the split returns that file BELOW its frozen number rather than raising it. Graded by real git check-ignore against a real repository with git's own global and system config neutralised, so a contributor's excludes file cannot make it pass or fail." + }, "tests/unit/test_merge_preflight_handler.py": 1139, "tests/unit/test_onboard_scanner_claude_memory.py": 562, "tests/unit/test_onboard_scanner_cli_toolchain.py": 504, diff --git a/src/bonfire/cli/commands/init.py b/src/bonfire/cli/commands/init.py index b378bebf..a4382db3 100644 --- a/src/bonfire/cli/commands/init.py +++ b/src/bonfire/cli/commands/init.py @@ -34,20 +34,40 @@ # sites stay consistent. _INIT_READ_MAX_BYTES: int = 1 * 1024 * 1024 -# ``.bonfire/`` carries a MIX of operator-local state (the per-machine -# ``tools.local.toml`` written by ``bonfire scan``) AND artefacts that -# ARE committable: ``.bonfire/sessions`` (handoff history), -# ``.bonfire/context.json`` (project config), ``.bonfire/vault`` -# (knowledge backend seed), ``.bonfire/costs.jsonl`` (cost ledger, when -# operator opts in to commit). A broad ``.bonfire/`` ignore would -# silently exclude those committable sub-paths and break workflows that -# depend on them landing in git. The narrower entry names the single -# operator-local file the W8.G work introduced so other sub-paths under -# ``.bonfire/`` remain stageable by default — a contract pinned by the -# gitignore-narrowness test in ``test_tools_section_is_local.py``. The -# operator can still add broader patterns to ``.gitignore`` by hand if -# they want; ``bonfire init`` does not assume that policy. +# ``.bonfire/`` carries a MIX of state that must never leave the +# operator's machine AND state that is committable by design. The seeded +# entries name ONLY the former, one line each. +# +# Committable — deliberately NOT ignored: ``.bonfire/sessions/`` (handoff +# history operators commit so the next session picks up the thread), +# ``.bonfire/context.json`` (project config, portable by design), +# ``.bonfire/costs.jsonl`` (cost ledger, committed when the operator opts +# in to keeping the spend record in the repo). +# +# Never committable — seeded here: +# * ``.bonfire/tools.local.toml`` — the host tool inventory and version +# footprint ``bonfire scan`` stamps; per-machine, and a privacy leak +# in a public repo. (Introduced by the W8.G migration.) +# * ``.bonfire/vault`` — the knowledge backend's store, once an +# operator enables a persistent one. NO trailing slash: one default +# path, TWO on-disk shapes, and one pattern must cover both. LanceDB +# makes it a DIRECTORY (a vector index of the operator's own source); +# SQLite hands it to ``sqlite3.connect``, making it a REGULAR FILE +# holding the indexed source as cleartext — the worse leak, and the +# shape a directory-only pattern cannot reach. Slash-less covers both. +# +# A broad ``.bonfire/`` ignore would be simpler and is deliberately NOT +# used: it would silently exclude the committable sub-paths above and +# break the workflows that depend on them landing in git. BOTH +# directions — the vault IS covered, no other ``.bonfire`` sub-path is — +# are pinned by ``tests/unit/test_init_gitignore_width.py``. An operator +# may still add broader patterns by hand; ``bonfire init`` does not +# assume that policy. ``_GITIGNORE_LINE`` keeps its name and +# single-string shape (the tools-file entry, imported by name from the +# hardening tests); ``_GITIGNORE_LINES`` is the full seeded sequence. _GITIGNORE_LINE = ".bonfire/tools.local.toml" +_GITIGNORE_VAULT_LINE = ".bonfire/vault" +_GITIGNORE_LINES: tuple[str, ...] = (_GITIGNORE_LINE, _GITIGNORE_VAULT_LINE) # --------------------------------------------------------------------------- @@ -184,13 +204,26 @@ def _make_directory_or_refuse(path: Path, label: str) -> None: def _ensure_gitignore_entry(target: Path, line: str) -> None: - """Append ``line`` to ``target/.gitignore`` iff not already present. + """Seed exactly one entry — the single-line primitive. + + Kept as the narrow entry point (one line in, one write out): that is + the shape the symlink/TOCTOU hardening tests drive directly. + """ + _ensure_gitignore_entries(target, (line,)) + + +def _ensure_gitignore_entries(target: Path, lines: tuple[str, ...]) -> None: + """Append each of ``lines`` to ``target/.gitignore`` iff absent. Idempotent: re-running ``bonfire init`` MUST NOT duplicate an entry (the no-duplicate canary pins this). The presence check matches a - stripped/non-comment line against the requested entry; existing - comments and blank lines are preserved. Creates ``.gitignore`` if - absent. + stripped line against each requested entry; existing comments and + blank lines are preserved. Creates ``.gitignore`` if absent. + + All missing entries land in ONE write — the fresh-file branch emits + the header plus every line, the extend branch appends the missing + lines as a single payload — so the number of writes is independent + of the number of entries. Uses :func:`safe_write_text` (W7.M) when creating the file fresh and :func:`safe_append_text` (W7.M append helper) when extending @@ -219,19 +252,21 @@ def _ensure_gitignore_entry(target: Path, line: str) -> None: _require_regular_file_slot(gitignore_path, ".gitignore") if not gitignore_path.exists(): - # Fresh file — create with the entry and a brief header so a + # Fresh file — create with the entries and a brief header so a # future contributor reading ``.gitignore`` understands why the - # operator-local file is excluded. - body = f"# Bonfire — operator-local state (do not commit).\n{line}\n" - _write_or_refuse(gitignore_path, body, label=".gitignore") + # operator-local paths are excluded. + header = "# Bonfire — operator-local state (do not commit).\n" + _write_or_refuse(gitignore_path, header + _as_entry_block(lines), label=".gitignore") return # W11 M2: route through ``safe_read_capped_text`` so the read uses # ``O_NOFOLLOW`` defense-in-depth against a race-planted symlink # between the ``is_symlink`` pre-check above and this read. existing = _read_or_refuse(gitignore_path, ".gitignore") - if line in [ln.strip() for ln in existing.splitlines()]: - # Already covered — idempotent no-op. + present = {ln.strip() for ln in existing.splitlines()} + missing = tuple(line for line in lines if line not in present) + if not missing: + # Every entry already covered — idempotent no-op. return # Append on a fresh line. Ensure exactly one trailing newline before @@ -251,7 +286,16 @@ def _ensure_gitignore_entry(target: Path, line: str) -> None: # is refused at ``open(2)`` time by the kernel rather than slipping # through to an attacker-controlled target. suffix = "" if existing.endswith("\n") else "\n" - _append_or_refuse(gitignore_path, suffix + f"{line}\n", label=".gitignore") + _append_or_refuse(gitignore_path, suffix + _as_entry_block(missing), label=".gitignore") + + +def _as_entry_block(lines: tuple[str, ...]) -> str: + """Render ``lines`` as newline-terminated ``.gitignore`` entries. + + One place decides that each seeded entry owns its line and ends in + ``\\n``, so the fresh-file body and the append payload cannot drift. + """ + return "".join(f"{line}\n" for line in lines) def _existing_gitignore_entries(gitignore_path: Path) -> set[str]: @@ -268,7 +312,7 @@ def _existing_gitignore_entries(gitignore_path: Path) -> set[str]: body = safe_read_capped_text(gitignore_path, max_bytes=_INIT_READ_MAX_BYTES) except (OSError, ValueError): return set() - return {ln.strip() for ln in body.splitlines()} & {_GITIGNORE_LINE} + return {ln.strip() for ln in body.splitlines()} & set(_GITIGNORE_LINES) def _has_legacy_tools_section(toml_path: Path) -> bool: @@ -348,9 +392,9 @@ def _report(target: Path, pre_existed: dict[str, bool], seeded_before: set[str]) Quick Start enumerated only the subset (``bonfire.toml`` + ``.bonfire/``) and the prior success message hid the rest: the ``agents/`` scaffold the prompt compiler reads from, and the - operator-local-state line appended to ``.gitignore``. A README - reconciliation test pins this list against the README so the two - cannot drift. + operator-local-state lines appended to ``.gitignore``. The README + reconciliation test only checks that four artefact tokens appear near + the README's ``bonfire init`` example, so it cannot notice a NEW one. Per-artefact verb prefix: ``Created:`` when the artefact was created this run, ``Already present:`` when it pre-existed. The artefact-name @@ -370,12 +414,15 @@ def _verb(existed: bool) -> str: f" - {_verb(pre_existed['agents_dir'])}: agents/ " "(role-local prompt + identity-block overrides)" ) - # The .gitignore entry is reported per-entry, not per-file, because - # the file may pre-exist with unrelated user content while the entry - # is freshly appended. Reporting "Already present" only when BOTH the - # file and the line existed before this run keeps the truth honest. - entry_existed = pre_existed["gitignore"] and _GITIGNORE_LINE in seeded_before - typer.echo(f" - {_verb(entry_existed)}: .gitignore entry: {_GITIGNORE_LINE}") + # The .gitignore entries are reported per-entry, not per-file: the + # file may pre-exist with user content while one entry is freshly + # appended and the other was already there. Each line's verb follows + # whether THAT entry was in the file before this run, so a fresh + # append never reads "Already present" and neither is a pre-existing + # entry announced as "Created". + for line in _GITIGNORE_LINES: + entry_existed = pre_existed["gitignore"] and line in seeded_before + typer.echo(f" - {_verb(entry_existed)}: .gitignore entry: {line}") def init( @@ -444,11 +491,9 @@ def init( _make_directory_or_refuse(bonfire_dir, ".bonfire/") _make_directory_or_refuse(agents_dir, "agents/") - # W8.G — seed .gitignore so ``.bonfire/tools.local.toml`` (and any - # future operator-local file under ``.bonfire/``) is never staged - # for commit. Idempotent: re-running ``bonfire init`` does not - # duplicate the entry. - _ensure_gitignore_entry(target, _GITIGNORE_LINE) + # Seed .gitignore for the operator-local paths (which ones, and why + # each, at ``_GITIGNORE_LINES``). Idempotent on re-run. + _ensure_gitignore_entries(target, _GITIGNORE_LINES) _report(target, pre_existed, seeded_before) raise typer.Exit(0) diff --git a/tests/unit/test_init_gitignore_width.py b/tests/unit/test_init_gitignore_width.py new file mode 100644 index 00000000..9cd5677c --- /dev/null +++ b/tests/unit/test_init_gitignore_width.py @@ -0,0 +1,284 @@ +# SPDX-License-Identifier: Apache-2.0 +# Copyright 2026 BonfireAI + +"""Contract: the WIDTH of the ``.gitignore`` that ``bonfire init`` seeds. + +This module owns ONE contract, graded in both directions. + + * No UNDER-coverage (the control rod). Every operator-local path the + seed exists to cover MUST be ignored by the seeded ``.gitignore`` + itself: the per-machine ``.bonfire/tools.local.toml`` written by + ``bonfire scan``, and the knowledge-backend store at + ``.bonfire/vault`` in BOTH on-disk shapes that one path can take. + Without this direction the whole class is satisfied by a seed that + covers nothing at all. + * No OVER-coverage. The committable sub-paths under ``.bonfire/`` MUST + stay stageable: ``sessions/`` handoffs (operators commit these so the + next session picks up the thread), ``context.json`` (portable project + config) and the opt-in ``costs.jsonl`` ledger. A bare ``.bonfire/`` + line is the obvious way to get this wrong, so it is also named + directly, and the seeded ``.bonfire`` entries are pinned as a closed + set — a THIRD distinct path would fail here even if it were narrow. + +Why the store needs two probes. ``bonfire.knowledge.get_vault_backend`` +documents one default ``vault_path`` for every backend, +``.bonfire/vault``. The LanceDB backend turns that path into a DIRECTORY +(a vector index built from the operator's own source). The SQLite backend +hands the same string to ``sqlite3.connect``, which creates a REGULAR +FILE whose ``vault_entries.content`` column holds that source as +cleartext — strictly worse to leak than embeddings. A trailing-slash +(directory-only) pattern covers the first shape and NOT the second, and +``git check-ignore``'s verdict on such a pattern depends on what is on +disk, so this module materialises the file shape rather than trusting a +name. + +Preventive, not the closing of a live leak. ``get_vault_backend`` has no +production caller today: ``engine/composition.py`` documents the ingest +consumer as deliberately not wired, and ``VaultConfig`` carries only +``session_dir`` and ``context_file``, so no ``bonfire.toml`` key selects +a backend. This pin is what makes the store safe to enable — it does not +report that anything is leaking now. + +Split out of ``test_tools_section_is_local.py`` (which keeps Pins #1-#8, +including the init-coverage and idempotence pins) so that one contract +owns one file. +""" + +from __future__ import annotations + +import os +import subprocess +from pathlib import Path + +import pytest + +# The exact set of non-comment ``.bonfire`` entries ``bonfire init`` is +# allowed to seed. Written as LITERAL strings on purpose: importing +# ``_GITIGNORE_LINES`` from ``init.py`` would make the closed-world +# assertion below true by construction — the gate would then grade +# nothing at all, no matter what init seeds. +_EXPECTED_SEEDED_BONFIRE_ENTRIES = {".bonfire/tools.local.toml", ".bonfire/vault"} + +# Blanket spellings that would cover every committable sub-path at once. +_BLANKET_SPELLINGS = {".bonfire", ".bonfire/", ".bonfire/*", ".bonfire/**"} + +# Neutralise the contributor's and the CI runner's own git configuration +# for EVERY git subprocess this module runs. ``git check-ignore`` reports +# matches from ``core.excludesFile`` as well as from the repo's +# ``.gitignore``, so a global excludes carrying ``*.json``, ``*.md`` or +# ``.bonfire/`` would otherwise decide this test's verdict on a file +# ``bonfire init`` never wrote. ``tests/conftest.py`` scrubs ``BONFIRE_*`` +# only; nothing else does this. Residual: git's XDG fallback +# (``~/.config/git/ignore``) is not disabled by these two variables, which +# is why every assertion below attributes the match to a SOURCE rather +# than merely asking whether one exists. +_GIT_ENV = { + **os.environ, + "GIT_CONFIG_GLOBAL": os.devnull, + "GIT_CONFIG_SYSTEM": os.devnull, +} + + +def _run_init(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Run ``bonfire init .`` inside *tmp_path* and require a clean exit. + + So no pin can mistake a crashed init for a narrow gitignore. This is a + verbatim twin of the helper in ``test_tools_section_is_local.py``: + sharing it would need either a third helper module or a + ``tests/conftest.py`` fixture, and a new import surface is a worse + trade than six duplicated lines whose meaning is fixed by the + assertion text they carry. + """ + from typer.testing import CliRunner + + from bonfire.cli.app import app + + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(app, ["init", "."]) + assert result.exit_code == 0, ( + f"bonfire init must succeed in {tmp_path}; got exit_code={result.exit_code}, " + f"output={result.output!r}" + ) + + +def _ignored_by(repo: Path, rel_path: str) -> str: + """Return the ``source:line:pattern`` that ignores *rel_path*, or ``""``. + + ``git check-ignore -v`` rather than ``-q``: the quiet form answers only + "something matched", which cannot tell the seeded ``.gitignore`` from an + unrelated global excludes file, and both directions below need the + source. + + Exit statuses: 0 one or more patterns matched, 1 none did, 128 a fatal + error (not a repo, bad option, unreadable index). Folding 128 into + ``""`` — "not ignored" — would make every must-NOT-be-ignored assertion + pass on a void while looking green, so the status is asserted first and + a fatal git error is a loud red instead of an alibi. + """ + check = subprocess.run( + ["git", "check-ignore", "-v", rel_path], + cwd=repo, + capture_output=True, + text=True, + env=_GIT_ENV, + ) + assert check.returncode in (0, 1), ( + f"git check-ignore could not answer for {rel_path!r} in {repo}: " + f"returncode={check.returncode} (0=ignored, 1=not ignored; anything " + f"else is a fatal git error, never a verdict). " + f"stderr={check.stderr!r} stdout={check.stdout!r}" + ) + return check.stdout.strip() if check.returncode == 0 else "" + + +class TestInitGitignoreCoversOperatorStateAndNothingElse: + """``bonfire init``'s seeded ``.gitignore`` is exactly as wide as it needs. + + Graded by real ``git check-ignore`` against a real repository, so the + contract is git's own pattern semantics rather than this test's idea of + them. + """ + + def test_seeded_gitignore_covers_operator_state_without_over_covering( + self, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """``git check-ignore`` grades both directions against a real repo.""" + _run_init(tmp_path, monkeypatch) + + gitignore_path = tmp_path / ".gitignore" + + # The skip below fires ONLY when git is absent or cannot create a + # repo at all — never as a way to absorb a real gitignore failure, + # and the reason names which of the two it was. + try: + subprocess.run( + ["git", "init", "-q"], + cwd=tmp_path, + check=True, + capture_output=True, + env=_GIT_ENV, + ) + except (FileNotFoundError, subprocess.CalledProcessError) as exc: + pytest.skip(f"git unavailable, cannot grade gitignore semantics: {exc!r}") + + # Materialise the SQLite shape: a REGULAR FILE at .bonfire/vault, + # exactly what ``sqlite3.connect(".bonfire/vault")`` produces. This + # is not decoration — a directory-only pattern's verdict DEPENDS on + # the on-disk shape (git will not apply a trailing-slash pattern to + # something it cannot see is a directory), so probing the name alone + # would grade a different question than the one operators face. + (tmp_path / ".bonfire" / "vault").write_bytes(b"") + + committable_paths = [ + ".bonfire/sessions/handoff.md", + ".bonfire/context.json", + ".bonfire/costs.jsonl", + # ``.bonfire/vault/seed.md`` was asserted committable here until + # it was measured: nothing under ``src/`` ever writes a + # ``seed.md`` and the string appeared in exactly one place in the + # whole repository — this assertion. It guarded a phantom. The + # vault is operator content either way, so the path moved to + # ``ignored_paths`` below. + ] + ignored_paths = [ + # Per-machine tool inventory written by ``bonfire scan``. + ".bonfire/tools.local.toml", + # The store's TWO shapes. The bare path is the SQLite shape + # (materialised above as a regular file); the nested paths are + # the LanceDB directory shape. Together they separate FILE from + # DIRECTORY, which is the distinction a trailing slash gets + # wrong. ``some-index.idx`` is deliberately synthetic — no code + # in this repo writes that name; it is here so the pattern is + # shown to cover ARBITRARY nesting under the directory, not one + # known filename. + ".bonfire/vault", + ".bonfire/vault/vault_v2.lance/data/0.lance", + ".bonfire/vault/some-index.idx", + ] + # Non-vacuity: an empty list makes its loop below pass while grading + # nothing, which is how this whole class goes quiet. + assert committable_paths, "committable_paths is empty: over-coverage check is vacuous" + assert ignored_paths, "ignored_paths is empty: the coverage control rod is vacuous" + + body = gitignore_path.read_text() + + # Direction 2 first: the control rod. Attribution to ``.gitignore:`` + # is what keeps an unrelated pattern in someone's global excludes + # (say ``*.lance``) from satisfying it over a seed covering nothing. + for rel_path in ignored_paths: + match = _ignored_by(tmp_path, rel_path) + assert match.startswith(".gitignore:"), ( + f"bonfire init's .gitignore at {gitignore_path} UNDER-covers: " + f"{rel_path!r} is not ignored by the SEEDED .gitignore, so " + f"`git add` would stage the operator's own content. " + f"check-ignore said {match!r} (empty = no pattern matched; a " + f"non-.gitignore source means some other excludes file, not " + f"the seed, would have to do the job). Gitignore body:\n{body}" + ) + + # Direction 1: no over-coverage. The verdict is about the SEEDED + # file, so only a ``.gitignore:``-sourced match is an accusation + # against ``bonfire init``. A foreign source is collected and + # reported separately, at the very end, so that a contributor's own + # excludes can never (a) be mistaken for bonfire over-covering, nor + # (b) pre-empt bonfire's verdict. + foreign_matches: list[str] = [] + for rel_path in committable_paths: + match = _ignored_by(tmp_path, rel_path) + assert not match.startswith(".gitignore:"), ( + f"bonfire init's .gitignore at {gitignore_path} OVER-covers: " + f"{rel_path!r} is matched by {match!r} but must remain " + f"stageable. Gitignore body:\n{body}" + ) + if match: + foreign_matches.append(f"{rel_path} <- {match}") + + seeded = [ + line.strip() + for line in body.splitlines() + if line.strip() and not line.strip().startswith("#") + ] + + # Blanket cover, named directly. The committable loop already rules + # it out, but asserting it by name makes the FAILURE readable + # instead of leaving the reader to infer which pattern was too wide. + blanket = _BLANKET_SPELLINGS & set(seeded) + assert not blanket, ( + f"bonfire init seeded a blanket .bonfire cover {sorted(blanket)!r} in " + f"{gitignore_path}, which ignores every committable sub-path under it. " + f"Seeded entries: {seeded!r}. Gitignore body:\n{body}" + ) + + # Closed-world width. The probe lists above are finite samples, so + # on their own they cannot see a THIRD seeded path (``.bonfire/cache/``, + # ``.bonfire/tmp*``) that no sample happens to touch. Set equality + # against a literal expectation is the width bound: a new seeded + # entry fails here and has to be argued for, and a dropped entry + # fails here too. + seeded_bonfire = {line for line in seeded if ".bonfire" in line} + assert seeded_bonfire == _EXPECTED_SEEDED_BONFIRE_ENTRIES, ( + f"the set of .bonfire entries bonfire init seeds into " + f"{gitignore_path} changed. Expected exactly " + f"{sorted(_EXPECTED_SEEDED_BONFIRE_ENTRIES)!r}; got " + f"{sorted(seeded_bonfire)!r} (added " + f"{sorted(seeded_bonfire - _EXPECTED_SEEDED_BONFIRE_ENTRIES)!r}, " + f"missing {sorted(_EXPECTED_SEEDED_BONFIRE_ENTRIES - seeded_bonfire)!r}). " + f"Every seeded path must be operator-local AND must not cover a " + f"committable sibling; if this change is correct, update the " + f"expectation here deliberately. Gitignore body:\n{body}" + ) + + # Environment signal, deliberately last and deliberately worded so + # it can never be read as an accusation against ``bonfire init``. + assert not foreign_matches, ( + f"THIS IS NOT A BONFIRE DEFECT: bonfire init's seed graded clean " + f"above. Some OTHER excludes source on this machine ignores a " + f"path bonfire deliberately leaves committable: {foreign_matches!r}. " + f"GIT_CONFIG_GLOBAL and GIT_CONFIG_SYSTEM are already pointed at " + f"{os.devnull} for this test, so the likely source is git's XDG " + f"fallback (~/.config/git/ignore) or .git/info/exclude. Fix the " + f"environment, or neutralise that source here too — do not widen " + f"or narrow the seed to satisfy it." + ) diff --git a/tests/unit/test_no_persona_names_in_public_docs.py b/tests/unit/test_no_persona_names_in_public_docs.py index b7fac808..699bb9c8 100644 --- a/tests/unit/test_no_persona_names_in_public_docs.py +++ b/tests/unit/test_no_persona_names_in_public_docs.py @@ -163,12 +163,12 @@ # `standard_build` bullet was corrected against the tree. ( "README.md", - 377, + 378, "predecessor named Passelewe. History is sacred — see", ), ( "README.md", - 378, + 379, "`docs/_lore/passelewe.md` if you want the lineage.", ), # --- CHANGELOG.md — predecessor-persona historical entries ----- @@ -194,22 +194,22 @@ # under [Unreleased]. Anchors only; expected text is unchanged. ( "CHANGELOG.md", - 662, + 677, "predecessor persona (Passelewe, the Chamberlain) was retired; the", ), ( "CHANGELOG.md", - 664, + 679, "`docs/_lore/passelewe.md`. The persona builtins directory", ), ( "CHANGELOG.md", - 665, + 680, "`src/bonfire/persona/builtins/passelewe/` was deleted; a new", ), ( "CHANGELOG.md", - 682, + 697, 'to ban `"passelewe"` in src/ (the predecessor persona is gone, so', ), # --- CLAUDE.md — constellation-pointer breadcrumbs -------------- diff --git a/tests/unit/test_tools_section_is_local.py b/tests/unit/test_tools_section_is_local.py index 8b58189d..e8665fcf 100644 --- a/tests/unit/test_tools_section_is_local.py +++ b/tests/unit/test_tools_section_is_local.py @@ -23,7 +23,13 @@ ``bonfire init`` time. ``bonfire.toml`` stays project-portable; ``.bonfire/tools.local.toml`` stays per-machine, never committed. -This file pins six contracts: +This file pins the six numbered contracts below, plus two +defense-in-depth pins documented at their own classes (#7, the reader's +symlink refusal; #8, the sentinel label whitelist). A ninth pin — the +WIDTH of the ``.gitignore`` ``bonfire init`` seeds, in both directions — +lives in ``tests/unit/test_init_gitignore_width.py``. + +The six: 1. ``generate_config`` MUST NOT include a ``[bonfire.tools]`` section in the main ``config_toml`` string. @@ -34,9 +40,10 @@ ``[bonfire.tools]`` table. 3. ``bonfire init`` MUST add a ``.gitignore`` entry covering - ``.bonfire/tools.local.toml`` (the simplest sufficient cover is the - directory itself, ``.bonfire/``, but a narrower entry that names - the file directly is equally acceptable). + ``.bonfire/tools.local.toml``. Any pattern that matches the file + under standard git semantics satisfies this pin — but the width + module independently forbids a blanket ``.bonfire/`` cover, so the + seed has to name the operator-local paths rather than the dir. 4. The reader API ``load_tools_config(project_path)`` (new module surface owned by the Warrior) MUST prefer @@ -107,6 +114,25 @@ def _fake_tool_scans() -> list[ScanUpdate]: return [_scan("cli_toolchain", name, ver) for name, ver in _FAKE_TOOLS] +def _run_init(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """Run ``bonfire init .`` inside *tmp_path* and require a clean exit. + + Shared by every init-facing pin below so there is one assertion text + for "init did not even succeed" — no pin can mistake a crashed init + for a narrow gitignore. + """ + from typer.testing import CliRunner + + from bonfire.cli.app import app + + monkeypatch.chdir(tmp_path) + result = CliRunner().invoke(app, ["init", "."]) + assert result.exit_code == 0, ( + f"bonfire init must succeed in {tmp_path}; got exit_code={result.exit_code}, " + f"output={result.output!r}" + ) + + def _load_tools_reader(): """Lazy-import the (new) reader so its absence fails per-test cleanly. @@ -305,11 +331,14 @@ def test_local_tools_file_not_created_when_no_tools_scans( class TestInitGitignoresLocalToolsFile: """``bonfire init`` MUST seed a ``.gitignore`` entry that prevents - ``.bonfire/tools.local.toml`` from ever being staged. The simplest - sufficient cover is the directory ``.bonfire/`` itself; a narrower - entry naming the file directly is equally acceptable. The Knight + ``.bonfire/tools.local.toml`` from ever being staged. This class accepts any pattern that matches the operator-local file under - standard git semantics. + standard git semantics, including the blanket ``.bonfire/`` cover — + it grades coverage, not width. Width is graded in + ``tests/unit/test_init_gitignore_width.py``, which asserts set + equality against the two entries the seed is allowed to hold and so + rules the blanket cover out; the two together admit only a seed that + names the operator-local paths. """ def test_init_creates_gitignore_covering_tools_local_file( @@ -320,16 +349,7 @@ def test_init_creates_gitignore_covering_tools_local_file( """``bonfire init`` adds an entry to ``.gitignore`` matching ``.bonfire/tools.local.toml``. """ - from typer.testing import CliRunner - - from bonfire.cli.app import app - - runner = CliRunner() - monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["init", "."]) - assert result.exit_code == 0, ( - f"init must succeed; got exit_code={result.exit_code}, output={result.output!r}" - ) + _run_init(tmp_path, monkeypatch) gitignore_path = tmp_path / ".gitignore" assert gitignore_path.exists(), ( @@ -372,36 +392,51 @@ def test_init_gitignore_idempotent_does_not_duplicate( tmp_path: Path, monkeypatch: pytest.MonkeyPatch, ) -> None: - """Running ``bonfire init`` twice (re-init) MUST NOT duplicate the - ``.bonfire/`` entry in ``.gitignore``. ``bonfire init`` is + """Running ``bonfire init`` twice (re-init) MUST NOT duplicate any + seeded ``.bonfire/`` entry in ``.gitignore``. ``bonfire init`` is idempotent (per ``init.py``'s ``if not toml_path.exists()`` and ``mkdir(exist_ok=True)``); the gitignore seeding must follow the - same rule — re-running must not append a second copy of the - same line. + same rule — re-running must not append a second copy of a line, + nor accumulate blank lines. + + The check is byte-level: the whole file must come back identical, + so a duplicate of ANY seeded entry fails and so does stray + whitespace. That is strictly stronger than the entry-count cap it + replaces ON IDEMPOTENCE and strictly weaker ON WIDTH — the old cap + ("at most one line mentioning ``.bonfire``") incidentally bounded + how many paths could be seeded, while forbidding a SECOND + legitimate operator-local entry, so it conflated the two + properties. Width is bounded on its own terms in + ``tests/unit/test_init_gitignore_width.py``: set equality against + a literal expectation, which a third seeded path fails. """ - from typer.testing import CliRunner - - from bonfire.cli.app import app - - runner = CliRunner() - monkeypatch.chdir(tmp_path) - - result1 = runner.invoke(app, ["init", "."]) - assert result1.exit_code == 0 - result2 = runner.invoke(app, ["init", "."]) - assert result2.exit_code == 0 - gitignore_path = tmp_path / ".gitignore" - assert gitignore_path.exists() - body = gitignore_path.read_text() - lines = [line.strip() for line in body.splitlines()] - # Count how many gitignore lines mention the .bonfire token. - bonfire_lines = [line for line in lines if ".bonfire" in line and not line.startswith("#")] - assert len(bonfire_lines) <= 1, ( - f"bonfire init duplicated .bonfire-related .gitignore entries on " - f"re-init. Got {len(bonfire_lines)} matching lines: " - f"{bonfire_lines!r}. Full body:\n{body}" + _run_init(tmp_path, monkeypatch) + after_first = gitignore_path.read_text() + _run_init(tmp_path, monkeypatch) + after_second = gitignore_path.read_text() + + assert after_second == after_first, ( + f"re-running bonfire init rewrote {gitignore_path}. After first run:\n" + f"{after_first}\n--- After second run:\n{after_second}" + ) + bonfire_lines = [ + line.strip() + for line in after_second.splitlines() + if ".bonfire" in line and not line.strip().startswith("#") + ] + # Non-vacuity: with no seeded entries at all, the duplicate check + # below would pass over an empty list. + assert bonfire_lines, ( + f"bonfire init seeded no .bonfire entry at all into {gitignore_path}, " + f"so the no-duplicate check has nothing to grade. Full body:\n{after_second}" + ) + duplicated = sorted({ln for ln in bonfire_lines if bonfire_lines.count(ln) > 1}) + assert not duplicated, ( + f"bonfire init duplicated a .bonfire .gitignore entry in " + f"{gitignore_path} on re-init: {duplicated!r}. All .bonfire lines: " + f"{bonfire_lines!r}. Full body:\n{after_second}" ) @@ -817,77 +852,17 @@ def test_sentinel_drops_uppercase_and_oversize_labels(self) -> None: # --------------------------------------------------------------------------- -# Pin #9 — Gitignore narrowness: committable sub-paths under .bonfire/ -# must remain stageable by default. +# Pin #9 — MOVED. The width of the ``.gitignore`` ``bonfire init`` seeds — +# both directions, plus the closed set of seeded ``.bonfire`` +# entries — now lives in its own module: +# +# tests/unit/test_init_gitignore_width.py +# +# It did not vanish and it did not weaken: the move added the +# SQLite file shape of ``.bonfire/vault`` to the covered paths, +# made a fatal ``git check-ignore`` exit a red instead of a +# silent "not ignored", attributed both directions to a source so +# a contributor's global excludes cannot decide the verdict, and +# replaced the sampled width check with set equality against a +# literal expectation. One contract, one file. # --------------------------------------------------------------------------- - - -class TestInitGitignoreDoesNotOverCover: - """``bonfire init`` must NOT seed a gitignore entry that excludes - committable sub-paths under ``.bonfire/`` (sessions, context.json, - vault seed, opt-in cost ledger). Over-broad coverage silently - breaks workflows that depend on those paths landing in git. - """ - - def test_gitignore_entry_does_not_cover_bonfire_sessions( - self, - tmp_path: Path, - monkeypatch: pytest.MonkeyPatch, - ) -> None: - """After init, the gitignore must NOT match - ``.bonfire/sessions/2026-05-15-handoff.md`` — operators commit - session handoffs. - """ - import subprocess - - from typer.testing import CliRunner - - from bonfire.cli.app import app - - runner = CliRunner() - monkeypatch.chdir(tmp_path) - result = runner.invoke(app, ["init", "."]) - assert result.exit_code == 0 - - # Use git itself to evaluate the gitignore (most authoritative). - # ``git check-ignore`` returns 0 when path IS ignored, 1 when - # NOT ignored. We want NOT ignored for these committable paths. - try: - subprocess.run( - ["git", "init", "-q"], - cwd=tmp_path, - check=True, - capture_output=True, - ) - except (FileNotFoundError, subprocess.CalledProcessError): - pytest.skip("git not available") - - committable_paths = [ - ".bonfire/sessions/handoff.md", - ".bonfire/context.json", - ".bonfire/vault/seed.md", - ".bonfire/costs.jsonl", - ] - for rel_path in committable_paths: - check = subprocess.run( - ["git", "check-ignore", "-q", rel_path], - cwd=tmp_path, - capture_output=True, - ) - # Exit code 1 = NOT ignored (good). - assert check.returncode == 1, ( - f"bonfire init's .gitignore over-covers: {rel_path!r} is " - f"matched by the seeded entry but should remain stageable. " - f"Gitignore body:\n{(tmp_path / '.gitignore').read_text()}" - ) - - # Sanity: the operator-local file IS ignored. - check = subprocess.run( - ["git", "check-ignore", "-q", ".bonfire/tools.local.toml"], - cwd=tmp_path, - capture_output=True, - ) - assert check.returncode == 0, ( - f"bonfire init's .gitignore did NOT cover the operator-local " - f"tools.local.toml file. Body:\n{(tmp_path / '.gitignore').read_text()}" - )