Fix/artifact id path traversal#150
Closed
philluiz2323 wants to merge 79 commits into
Closed
Conversation
…ouchdev#76) crystallize wrote a durable Page directly through store.put_page, embedding sess.task, sess.note, and sess.agent verbatim into the rendered markdown body. The body was never gated by propose_page + approve, so an agent calling kb.session_start(task=<payload>) and getting any one claim approved via crystallize could land arbitrary content into pages/. The page surfaces in kb.read_page, kb.list_pages, kb.context, and (once vouchdev#60 is fixed) kb.search. Restrict the summary body to fields the proposing agent cannot influence: session id (server-generated), timestamps (server clock), and the list of approved artifact ids. The agent-controlled fields remain on the Session model itself and are still queryable, but no longer promoted into a durable Page. Also include summary_page_id in the session.crystallize audit event's object_ids when a page is written, so vouch audit truthfully attributes the write. Adds two regression tests: - test_crystallize_summary_page_does_not_leak_agent_controlled_fields - test_crystallize_audit_event_records_summary_page_id
Leftover from the merge of main (which brought PR vouchdev#62's test_sessions.py changes); ruff F401 was failing CI on PR vouchdev#77.
…index FTS5 on update (vouchdev#78) Two compounding bugs let archived / superseded / redacted claims keep flowing back to agents through kb.context: 1. build_context_pack had no claim.status filter, so any retrieval hit was appended to the pack regardless of subsequent lifecycle mutations. 2. store.update_claim refreshed the embedding cache but never re-indexed the FTS5 row, so claims_fts.status stayed frozen at first-index time. Every lifecycle op (archive, supersede, contradict, confirm) routes through update_claim and was therefore invisible to FTS5 / semantic search. This made ClaimStatus.{ARCHIVED, SUPERSEDED, REDACTED} purely decorative on the read side — agents kept quoting retracted knowledge as if it were live. Fix: - storage.update_claim now also calls index_db.index_claim under a short-lived sqlite connection (mirroring proposals.approve's first-index pattern). FTS5 errors are logged-and-skipped so a lifecycle op never fails because the embedding/FTS5 layer is misconfigured. - context.build_context_pack resolves each claim hit, drops it if the on-disk status is in {ARCHIVED, SUPERSEDED, REDACTED}, and drops it if the claim YAML has been deleted between index time and now. CONTESTED claims keep surfacing so contradictions remain visible. Tests: - test_context_pack_excludes_archived_claims - test_context_pack_excludes_superseded_claims - test_update_claim_refreshes_fts5_status (asserts the FTS5 status column via direct SQL against state.db) No on-disk-layout, schema, or bundle-format change.
Resolves conflicts in CHANGELOG.md, tests/test_sessions.py, and auto-merges src/vouch/sessions.py with the recently-merged: - vouchdev#61 (FTS5 indexing of crystallize summary page) — adds index_db.index_page(...) right after store.put_page(...) inside crystallize. Composes cleanly with this PR's strict-derivation _build_summary_body and the summary_page_id audit fix. - vouchdev#62 (test_crystallize_collects_approval_failures) — preserved as a sibling test. - vouchdev#75 (bundle import sha256 verification) — CHANGELOG entry only. - vouchdev#73 (bundle POSIX separators) — CHANGELOG entry only. Both regression tests added by this PR (test_crystallize_summary_page_does_not_leak_agent_controlled_fields and test_crystallize_audit_event_records_summary_page_id) still pass against the merged sessions.crystallize.
vouchdev#81) The 'claims must cite sources' guarantee (README §'Why this exists' point 3; CONTRIBUTING §'Things we won't merge') used to live only in proposals.propose_claim, so every other write path silently accepted Claim(evidence=[]) and landed an uncited claim: - store.put_claim direct: existence-check loop iterates zero times. - store.update_claim: writes the YAML without re-validating. - bundle.import_apply via _validate_content: defers to Claim.model_validate, which accepted evidence=[] because the model had no min-length constraint. Add @field_validator('evidence') on Claim — raises ValueError when the list is empty. Closes all three bypass paths in one place. store.update_claim additionally re-validates via Claim.model_validate(claim.model_dump()) before persisting, so in-place mutation (c.evidence = []; store.update_claim(c)) raises before the YAML hits disk — the field validator only fires at construction time, not on attribute assignment. Four regression tests: - test_claim_model_rejects_empty_evidence (tests/test_storage.py) — Claim(evidence=[]) raises pydantic.ValidationError. - test_put_claim_rejects_empty_evidence — store.put_claim raises; no claims/<id>.yaml is written. - test_update_claim_rejects_empty_evidence — in-place mutation + update_claim raises; the on-disk YAML is unchanged. - test_import_rejects_uncited_claim (tests/test_bundle.py) — a schema-valid bundle whose claim YAML has evidence: [] is rejected by import_check (schema validation issue) and import_apply raises before writing. The existing guard in proposals.propose_claim becomes a redundant user-facing error message and is left in place for the friendlier CLI/JSONL error string. No on-disk-layout, schema, or bundle-format change; data the model never should have accepted now raises.
…ing from tarball (vouchdev#80) import_check previously only verified that tar members listed in the manifest had matching hashes, but never checked the reverse: whether every manifest entry had a corresponding tar member. A bundle whose manifest.json referenced claims/c1.yaml but whose tarball contained only manifest.json would pass import_check with ok=True, and import_apply would silently write nothing — no exception, no audit event indicating data loss. Add the missing-member pass (mirroring the existing check in export_check) so that manifest entries without a matching tar member produce a "manifest lists missing file" issue. import_apply then refuses to import because check.issues is non-empty.
read-only `vouch diff <old> <new>` that shows what changed between two claim or two page revisions: field-level changes plus a line-diff of the long text/body. auto-detects kind, hides churning metadata.
new read-only `vouch diff <old> <new>` compares two claims or two pages by id and shows what changed: scalar/list fields as old → new, plus a line-diff of the long text/body. auto-detects kind, hides churning metadata (timestamps, approved_by), supports --json. closes a roadmap 0.1 item.
mypy inferred old/new as Claim from the first branch, so the page branch's reassignment tripped an incompatible-assignment error in CI. annotate the union up front.
feat(diff): add `vouch diff` for claim/page revisions
…hdev#82 review) The new Claim.evidence min-citation validator (vouchdev#81) also fires when claims are read back from disk. A KB that has a pre-existing uncited claims/<id>.yaml from before the fix would otherwise crash vouch lint / vouch doctor with a bare pydantic.ValidationError deep in store.list_claims(). Add _load_claims_for_lint(), a per-file iteration that catches pydantic.ValidationError (and any other load error) and surfaces each bad file as a Finding with code='invalid_claim' and an explicit repair hint: 'edit the YAML to add a citation, or delete the file'. lint() also stops calling status() to populate counts — status() calls the strict store.list_claims() which would re-raise on the same files — and builds the counts dict inline from the safely-loaded valid claims. Regression test in tests/test_health.py: - test_lint_surfaces_legacy_uncited_claim_yaml_without_crashing hand-crafts a claims/legacy.yaml with evidence: [] (matches the on-disk shape an older buggy write path would have left), asserts vouch lint runs to completion, surfaces invalid_claim in findings with the repair-hint message, and that the well-formed sibling claim is still discovered. CHANGELOG migration note expanded to describe the repair hint.
The new inline-built counts in lint() (from 5c881de) is a literal dict with mixed value types (str/int/bool), which mypy correctly inferred as dict[str, object] and rejected against the HealthReport.counts: dict[str, int] annotation. The original status() returned the same mixed dict via an untyped 'dict' return, which masked the mismatch — the narrow type was effectively never checked at the call site. Widen counts to dict[str, Any] to match runtime reality. Also tighten status()'s return annotation from 'dict' to 'dict[str, Any]' for consistency. No caller does arithmetic on counts values; they just echo or pass through, so the widening is risk-free.
feat: add guided proposal review CLI
Feat/pending json
…ch-sync Feat/deterministic vouch sync
…ummary-page-bypass fix(sessions): close crystallize review-gate bypass via summary page
…s-citation fix(models): require Claim.evidence to be non-empty at the model layer
…nd-config fix(context): honor retrieval.backend config instead of hardcoding embeddings
feat(cli): vouch approve accepts multiple ids for scriptable bulk approval
…oposals Feat/batch approve proposals
…ontent-addressing # Conflicts: # CHANGELOG.md # tests/test_bundle.py # tests/test_cli.py
generalize init seeding into a named-template registry and ship a gittensor (SN74) starter pack as the first non-default template. complements gittensory rather than cloning its live-data signals.
generalize init seeding into a small in-code template registry. default 'starter' is unchanged; new 'gittensor' template seeds a cited, approved pack (1 source, 1 entity, 4 claims) about SN74 contribution scoring so a fresh kb in a gittensor repo has retrievable context immediately. unknown --template gives a clean error listing available packs. complements gittensory's live-data signals rather than cloning them.
# Conflicts: # src/vouch/health.py # tests/test_cli.py
# Conflicts: # src/vouch/cli.py
feat(init): add --template with a Gittensor (SN74) starter pack
…late feat(init): add --template with a gittensor seed pack
…chdev#97) Adds a `vouch.logging_config` module with a `JsonFormatter` and an idempotent `configure_logging()` that the CLI, MCP server, and JSONL server entry points call at startup. When `VOUCH_LOG_FORMAT=json`, the `vouch` logger emits one JSON object per line with `level`, `logger`, `event`, and any structured extras (`actor`, `object_ids`, ...) passed via the stdlib `extra=` argument. Any other value (including unset) is a no-op - vouch's loggers keep stdlib default behaviour, so callers who don't opt in see no change. Scoped to the `vouch` logger namespace with propagate=False in JSON mode, so this never reformats logs emitted by libraries vouch happens to depend on.
# Conflicts: # CHANGELOG.md
…tion-docs docs(gittensor): adoption guide (init, MCP wiring, decision workflow)
…le-proposals Feat/vouch expire stale proposals
the var was already documented in adapters/generic-mcp/README ('force a
specific KB root, skip discovery') but never wired into the code. now
`discover_root()` reads VOUCH_KB_PATH first and short-circuits the upward
walk, with a clear error if the value isn't an existing .vouch directory.
closes the doc-vs-code drift and removes the per-host 'cwd: ...' ceremony
needed by Claude Desktop (and any other host that launches the stdio
server from a default working directory).
fix(storage): honour VOUCH_KB_PATH env var in discover_root
Address review on vouchdev#112. - fsck() now loads claims via _load_claims_for_lint() like lint(), so a single invalid YAML (e.g. a legacy uncited claim from before vouchdev#81) becomes an `invalid_claim` finding instead of aborting the whole check with a traceback — the exact inconsistency a deep checker should surface. Counts are built from the safely-loaded claims via a shared _safe_counts() helper so the report can't re-trip the strict loader. - Add _drift_findings(kind, indexed_ids, on_disk_ids, findings) to emit the orphan + missing-row findings, collapsing the three near-identical blocks in _check_index_drift. Codes and messages are unchanged; adding a kind is now a one-liner. Add test_fsck_surfaces_invalid_claim_yaml_without_crashing.
Replace the _vouch_managed attribute (and its type: ignore[attr-defined]) with a _VouchManagedHandler(StreamHandler) subclass. configure_logging() identifies its handler by isinstance for install/reuse/remove, which reads cleaner and drops the attribute hack. Tests updated to match.
# Conflicts: # CHANGELOG.md
feat(health): vouch fsck — deep consistency checks beyond doctor
…t-json feat: structured JSON logging via VOUCH_LOG_FORMAT=json
…ent-addressing fix: enforce Source content-addressing on bundle/sync import
📝 WalkthroughWalkthroughThis PR implements a major feature release (0.1.0) spanning proposal lifecycle improvements, knowledge-base synchronization, Gittensor template support, structured logging, storage security hardening, and operational enhancements. All changes are tested and documented comprehensively. ChangesComprehensive 0.1.0 Release
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
CHANGELOG.md (1)
66-152:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd the artifact-id traversal hardening to the 0.1.0 release notes.
This block documents tar-member traversal and bundle integrity, but it still omits the model/storage guard that blocks
Claim(id="../escape")-style writes and the legacy poisoned-id lint behavior called out in the PR objective. That leaves the main security fix in this cohort undocumented for upgraders.🤖 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 `@CHANGELOG.md` around lines 66 - 152, The changelog omits the artifact-id traversal/storage hardening that prevents writes with poisoned ids (e.g., Claim(id="../escape")) and the legacy poisoned-id lint behavior; update the 0.1.0 release notes to mention the model/storage guard (Claim validation that rejects path-traversal ids), the store-side checks in store.put_claim / store.update_claim that enforce safe ids, and the vouch lint change that reports poisoned-id files rather than crashing (describe behavior of the new lint finding and migration guidance for affected KBs).src/vouch/bundle.py (1)
416-420:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail the import on write-time validation drift.
import_check()now rejects bad source content-addressing, butimport_apply()only reruns_validate_content()here, skips invalid members, and still records a successful import. That leaves a TOCTOU hole: a bundle changed between the check pass and this second tar read can bypass the new source invariant or silently drop tampered members while the audit log still saysbundle.importsucceeded.Suggested fix
val_issues: list[str] = [] _validate_content(member.name, body, val_issues) + _check_source_content_address(member.name, body, val_issues) if val_issues: - skipped.append(member.name) - continue + raise RuntimeError(f"refusing to import: {val_issues[0]}")🤖 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 `@src/vouch/bundle.py` around lines 416 - 420, import_apply currently ignores validation failures from _validate_content by appending to skipped and continuing, which allows TOCTOU tampering to pass; instead, make import_apply fail the whole import when val_issues is non-empty: remove the skipped.append(member.name)/continue behavior and raise a clear exception (e.g., ValueError or a specific ImportValidationError) including member.name and val_issues after calling _validate_content, so the caller records the import as failed (and adjust any surrounding try/except to propagate the error to the audit/logging path). Ensure the error type and message reference import_apply, member.name, and val_issues for easy debugging.src/vouch/context.py (1)
217-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFilter
backend/explainfrom the surviving items, not raw hits.
itemsnow correctly drops archived/superseded/redacted claims, butresult["backend"]and especiallyresult["explain"]are still derived from the unfilteredhits. Withexplain=True, retracted claim ids and scores still leak back to the caller, which bypasses the lifecycle guard added above.🤖 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 `@src/vouch/context.py` around lines 217 - 223, result["backend"] and result["explain"] are currently built from raw hits (leaking retracted/archived items); instead, compute these values from the filtered surviving items. Build a lookup (e.g., id_to_meta) from hits mapping claim id -> backend and score, then set result["backend"] using the backend for the first surviving item in items (or "none" if no items), and build result["explain"] by iterating over items and pulling kind/id/score/backend from the lookup so only surviving items are reported; update references to result["backend"] and result["explain"] accordingly.
🤖 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.
Inline comments:
In `@docs/superpowers/specs/2026-05-25-vouch-diff-design.md`:
- Around line 63-73: The fenced diff example in the docs lacks a language tag
which triggers markdownlint-cli2; update the triple-backtick fence that wraps
the diff block (the example showing "diff claim <old> → <new>" with
status/confidence/evidence/text lines) to include an explicit language
identifier (e.g., use "text" or "console") so the linter recognizes the block
while keeping the content unchanged.
In `@docs/superpowers/specs/2026-05-27-init-templates-design.md`:
- Around line 15-17: The fenced CLI example containing the line `vouch init
[--path P] [--template starter|gittensor]` needs a language tag to satisfy
markdownlint MD040; update the opening fence for that code block (the triple
backticks before the `vouch init` example) to include a language such as bash or
console (e.g., replace ``` with ```bash) so the fenced CLI example is explicitly
labeled.
In `@README.md`:
- Line 278: The README line claiming "multi-agent sync" is not implemented is
now stale; update that sentence to remove or rephrase the "multi-agent sync"
limitation and instead note that multi-agent synchronization is supported via
the new CLI commands `sync-check` and `sync-apply` (and link or reference the
multi-agent guide). Locate the sentence containing "multi-agent sync" in the
Pre-1.0 section and edit it to either drop that limitation or replace it with a
short note pointing readers to the multi-agent guide and the
`sync-check`/`sync-apply` workflow.
- Line 264: The CLI capability row in the README table is missing shipped
commands and must be updated to match the documented CLI surface: modify the CLI
row (the table cell containing `init`, `discover`, `capabilities`, ..., `serve`)
to include the missing commands `review`, `expire`, `sync-check`, `sync-apply`,
and `diff` so the "What ships today" summary reflects the actual shipped
commands; ensure the final comma/spacing formatting matches the existing list
style.
In `@src/vouch/cli.py`:
- Around line 472-499: Detect duplicate proposal ids in proposal_ids before any
check_approvable/do_approve work and reject the CLI invocation if any duplicates
exist: use collections.Counter to find ids with count > 1, emit an error line
for each duplicate (e.g. click.echo(f"✗ {pid}: duplicate proposal id",
err=True)) and raise click.ClickException listing the duplicate ids so the
command exits without mutating state; add this check before the existing
keep_going / check_approvable block (referencing proposal_ids, keep_going,
check_approvable, and do_approve).
In `@src/vouch/context.py`:
- Around line 148-154: The code currently only skips ArtifactNotFoundError from
store.get_claim(hid) but must also treat claim validation/load failures as
non-retrievable; update the try/except around store.get_claim(hid) to also catch
the specific exception(s) raised when a claim file is invalid (e.g.,
ClaimValidationError or ClaimLoadError — or the concrete exception your loader
raises) and continue on those exceptions just like ArtifactNotFoundError,
leaving the subsequent logic that checks claim.status against
_RETRACTED_CLAIM_STATUSES and uses claim.evidence unchanged; add a regression
test (e.g., test_indexed_invalid_claim_skipped) that inserts an
indexed-but-invalid claim YAML causing get_claim to raise and asserts the
context build completes and that the bad HID is ignored.
In `@src/vouch/diff.py`:
- Around line 75-84: The diff_artifacts function currently calls _kind_of per id
and rejects on the first differing kind; instead, implement pairwise kind
resolution: in diff_artifacts (and using KBStore, ArtifactDiff, DiffError) first
check whether both ids are claims (e.g., a helper like _exists_kind(store, id,
"claim") or calling _kind_of but comparing existence) and if so treat
kind="claim"; else check whether both are pages and if so treat kind="page";
only if neither pair matches, raise DiffError(s) that reflect unknown
artifact(s) or mismatched kinds—report unknown for an id only if it truly
doesn't exist in any namespace and otherwise return the appropriate cannot-diff
message when kinds differ. Ensure you update references in diff_artifacts to use
this pairwise resolution logic rather than per-id early returns.
In `@src/vouch/health.py`:
- Around line 84-114: The current safe-load pattern in _load_claims_for_lint
only protects Claim model loading; extend this to Page, Entity, Relation,
Evidence, Session, and Proposal by extracting the try/except per-file logic into
a reusable helper (or replicate the pattern) that iterates files under
store.kb_dir / "<kind>s", calls model.model_validate(_yaml_load(...)) for each,
appends valid models to the appropriate list, and converts ValidationError into
a Finding with code "invalid_<kind>" and other exceptions into
"unreadable_<kind>"; update the lint()/fsck() callers (and the loading blocks
currently for pages/entities/relations/evidence/sessions/proposals) to use this
safe loader so a single invalid file yields a Finding instead of aborting the
sweep.
In `@src/vouch/logging_config.py`:
- Around line 119-126: The JSON logging branch currently adds a handler but
leaves the vouch logger at WARNING so INFO records are still filtered; update
the JSON branch in logging_config.py (the block where selected == "json"
handling occurs, referencing logger and _VouchManagedHandler/JsonFormatter) to
explicitly lower the logger level to include INFO (e.g., set
logger.setLevel(logging.INFO) or logger.setLevel(os.environ-configurable level)
and ensure the handler's level permits those records) so vouch.info() messages
are emitted to the JSON handler.
In `@src/vouch/proposals.py`:
- Around line 371-387: The function expire_pending_after_days currently treats
booleans as integers because isinstance(..., int) accepts bool; update the
checks to reject booleans by using exact type checks: in the override branch
ensure type(override) is int and override >= 0 before returning it (otherwise
fall through to config/default), and replace isinstance(days, int) with
type(days) is int and days >= 0; keep returning _DEFAULT_EXPIRE_PENDING_DAYS for
all other cases. Reference: expire_pending_after_days, KBStore,
_DEFAULT_EXPIRE_PENDING_DAYS.
In `@src/vouch/server.py`:
- Around line 431-432: kb_expire() currently calls expire_pending(...,
expired_by=EXPIRE_ACTOR) which overwrites the transport-supplied caller (e.g.,
VOUCH_AGENT) so downstream lifecycle/audit fields lose operator identity; change
kb_expire() to pass the actual incoming caller/actor into expire_pending via the
expired_by parameter (use the actor value supplied to kb_expire() or the
transport context) instead of EXPIRE_ACTOR so the original caller is preserved
through expire_pending and lifecycle/audit.
In `@src/vouch/storage.py`:
- Around line 370-399: update_claim currently revalidates Claim shape but omits
the citation/source existence checks that put_claim performs, so mutated
claim.evidence can reference unknown artifacts; before writing and storing,
replicate the same existence validation loop used in put_claim (or extract it
into a shared helper) to verify each evidence/source id exists and raise
ArtifactNotFoundError for any missing id, then proceed with persistence,
_embed_and_store and FTS5 reindexing as before.
In `@src/vouch/sync.py`:
- Around line 148-154: _in _load_bundle_source, wrap the tarfile.open(...) call
in a try/except that catches tarfile.ReadError and re-raises a normalized error
(e.g., raise RuntimeError(f"invalid bundle file: {source_path}") from exc) so
invalid/regular files produce the same user-facing sync error the CLI expects;
keep the existing manifest lookup and JSON parsing logic intact and preserve the
original exception as the __cause__ when re-raising.
In `@tests/test_cli_output.py`:
- Around line 30-56: The tests test_status_no_color_by_default_in_pipe,
test_status_force_color_emits_ansi, and test_no_color_wins_over_force_color call
CliRunner().invoke(cli, ["status"]) which defaults to color=False and can strip
ANSI sequences; update each invocation to pass color=True (i.e.,
CliRunner().invoke(cli, ["status"], color=True) ) so the result.output preserves
ANSI escape sequences and the "\x1b[" assertions reliably test
FORCE_COLOR/NO_COLOR precedence.
In `@tests/test_templates.py`:
- Around line 78-82: Update test_init_unknown_template_clean_error to assert the
CLI error includes the list of valid template names in addition to the current
checks: after invoking cli in that test, import or call the source of truth for
templates (e.g. AVAILABLE_TEMPLATES constant or get_available_template_names()
used by cli) and assert that each expected template name (or the joined list
string) appears in res.output (case-insensitive) so the failure message shows
the available-template guidance.
---
Outside diff comments:
In `@CHANGELOG.md`:
- Around line 66-152: The changelog omits the artifact-id traversal/storage
hardening that prevents writes with poisoned ids (e.g., Claim(id="../escape"))
and the legacy poisoned-id lint behavior; update the 0.1.0 release notes to
mention the model/storage guard (Claim validation that rejects path-traversal
ids), the store-side checks in store.put_claim / store.update_claim that enforce
safe ids, and the vouch lint change that reports poisoned-id files rather than
crashing (describe behavior of the new lint finding and migration guidance for
affected KBs).
In `@src/vouch/bundle.py`:
- Around line 416-420: import_apply currently ignores validation failures from
_validate_content by appending to skipped and continuing, which allows TOCTOU
tampering to pass; instead, make import_apply fail the whole import when
val_issues is non-empty: remove the skipped.append(member.name)/continue
behavior and raise a clear exception (e.g., ValueError or a specific
ImportValidationError) including member.name and val_issues after calling
_validate_content, so the caller records the import as failed (and adjust any
surrounding try/except to propagate the error to the audit/logging path). Ensure
the error type and message reference import_apply, member.name, and val_issues
for easy debugging.
In `@src/vouch/context.py`:
- Around line 217-223: result["backend"] and result["explain"] are currently
built from raw hits (leaking retracted/archived items); instead, compute these
values from the filtered surviving items. Build a lookup (e.g., id_to_meta) from
hits mapping claim id -> backend and score, then set result["backend"] using the
backend for the first surviving item in items (or "none" if no items), and build
result["explain"] by iterating over items and pulling kind/id/score/backend from
the lookup so only surviving items are reported; update references to
result["backend"] and result["explain"] accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d6fd9250-c621-4e6e-85fc-b7072d377c34
📒 Files selected for processing (38)
CHANGELOG.mdREADME.mdROADMAP.mddocs/PROJECT-INDEX.mddocs/README.mddocs/gittensor.mddocs/multi-agent.mddocs/review-gate.mddocs/superpowers/specs/2026-05-25-vouch-diff-design.mddocs/superpowers/specs/2026-05-27-init-templates-design.mdsrc/vouch/bundle.pysrc/vouch/capabilities.pysrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/diff.pysrc/vouch/health.pysrc/vouch/jsonl_server.pysrc/vouch/logging_config.pysrc/vouch/models.pysrc/vouch/onboarding.pysrc/vouch/proposals.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/storage.pysrc/vouch/sync.pytests/test_bundle.pytests/test_cli.pytests/test_cli_output.pytests/test_context.pytests/test_diff.pytests/test_expire.pytests/test_health.pytests/test_logging.pytests/test_retrieval_backend.pytests/test_sessions.pytests/test_storage.pytests/test_sync.pytests/test_templates.py
Comment on lines
+63
to
+73
| ``` | ||
| diff claim <old> → <new> | ||
| status: working → stable | ||
| confidence: 0.7 → 0.9 | ||
| evidence: ['s1'] → ['s1', 's2'] | ||
| text: | ||
| --- a | ||
| +++ b | ||
| -old wording | ||
| +new wording | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to this fenced example.
markdownlint-cli2 flags this block because the fence has no language. text or console would satisfy the lint rule without changing the content.
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 63-63: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/specs/2026-05-25-vouch-diff-design.md` around lines 63 - 73,
The fenced diff example in the docs lacks a language tag which triggers
markdownlint-cli2; update the triple-backtick fence that wraps the diff block
(the example showing "diff claim <old> → <new>" with
status/confidence/evidence/text lines) to include an explicit language
identifier (e.g., use "text" or "console") so the linter recognizes the block
while keeping the content unchanged.
Comment on lines
+15
to
+17
| ``` | ||
| vouch init [--path P] [--template starter|gittensor] | ||
| ``` |
There was a problem hiding this comment.
Add a language to the fenced CLI example.
This block triggers the reported markdownlint failure (MD040).
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 15-15: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 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 `@docs/superpowers/specs/2026-05-27-init-templates-design.md` around lines 15 -
17, The fenced CLI example containing the line `vouch init [--path P]
[--template starter|gittensor]` needs a language tag to satisfy markdownlint
MD040; update the opening fence for that code block (the triple backticks before
the `vouch init` example) to include a language such as bash or console (e.g.,
replace ``` with ```bash) so the fenced CLI example is explicitly labeled.
Comment on lines
+55
to
+67
| ### `seed_gittensor_kb(store, approved_by) -> SeedResult` | ||
| Seeds, idempotently and approved-direct: | ||
| - **1 Source** — SN74 description text (`title="Gittensor SN74"`, | ||
| `locator="vouch:template/gittensor"`, `source_type="message"`, | ||
| `media_type="text/markdown"`), content from Gittensor's public README facts. | ||
| - **1 Entity** — `gittensor-sn74` (`type=project`). | ||
| - **4 Claims** (`type=fact`, `status=stable`, each citing the source, linked to | ||
| the `gittensor-sn74` entity): | ||
| 1. Miners earn TAO for pull requests merged into whitelisted repositories. | ||
| 2. Validators verify GitHub account ownership via a fine-grained PAT before scoring. | ||
| 3. Contributions are scored by code quality, repository allocation, and language factors. | ||
| 4. Sybil-resistant: GitHub account verification + merged-PR requirement prevent gaming. | ||
| - Each artifact keyed by a stable id; existence checks make re-runs no-ops. |
There was a problem hiding this comment.
Sync the spec with the shipped template contents.
This doc says the gittensor pack seeds 4 claims, but tests/test_templates.py now expects 7 claims and names additional seeded claim ids. Leaving the design doc behind the tested behavior will mislead the next person touching onboarding or docs.
Also applies to: 86-93
| |------|-----------------| | ||
| | Knowledge base | `.vouch/` folder, YAML claims/entities/relations/evidence/sessions, markdown pages with frontmatter, JSONL audit log, content-addressed sources | | ||
| | CLI | `init`, `discover`, `capabilities`, `status`, `lint`, `doctor`, `pending`, `show`, `approve`, `reject`, `propose-{claim,page,entity,relation}`, `source add`, `source verify`, `supersede`, `contradict`, `archive`, `confirm`, `cite`, `session {start,end}`, `crystallize`, `search`, `context`, `index`, `audit`, `export`, `export-check`, `import-check`, `import-apply`, `serve` | | ||
| | CLI | `init`, `discover`, `capabilities`, `status`, `lint`, `doctor`, `fsck`, `pending`, `show`, `approve`, `reject`, `propose-{claim,page,entity,relation}`, `source add`, `source verify`, `supersede`, `contradict`, `archive`, `confirm`, `cite`, `session {start,end}`, `crystallize`, `search`, `context`, `index`, `audit`, `export`, `export-check`, `import-check`, `import-apply`, `serve` | |
There was a problem hiding this comment.
Update the capability table to match the documented CLI surface.
This row is now missing shipped commands that are documented above in the same README, including review, expire, sync-check, sync-apply, and diff. Readers scanning “What ships today” will get an outdated surface summary.
🤖 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 `@README.md` at line 264, The CLI capability row in the README table is missing
shipped commands and must be updated to match the documented CLI surface: modify
the CLI row (the table cell containing `init`, `discover`, `capabilities`, ...,
`serve`) to include the missing commands `review`, `expire`, `sync-check`,
`sync-apply`, and `diff` so the "What ships today" summary reflects the actual
shipped commands; ensure the final comma/spacing formatting matches the existing
list style.
| ## Status | ||
|
|
||
| Pre-1.0. What's *not* in this implementation: vector embeddings (BM25/FTS5 only), per-runtime adapter templates, benchmark fixtures, multi-agent sync, scopes beyond a single field on Claim/Source. If a hole matters to you, file an issue. | ||
| Pre-1.0. What's *not* in this implementation: per-runtime adapter templates, benchmark fixtures, multi-agent sync, scopes beyond a single field on Claim/Source. If a hole matters to you, file an issue. |
There was a problem hiding this comment.
Remove the stale “multi-agent sync” limitation.
This line still says multi-agent sync is not implemented, but this PR adds sync-check / sync-apply and the multi-agent guide now documents the workflow. Leaving both claims in the docs is contradictory.
🤖 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 `@README.md` at line 278, The README line claiming "multi-agent sync" is not
implemented is now stale; update that sentence to remove or rephrase the
"multi-agent sync" limitation and instead note that multi-agent synchronization
is supported via the new CLI commands `sync-check` and `sync-apply` (and link or
reference the multi-agent guide). Locate the sentence containing "multi-agent
sync" in the Pre-1.0 section and edit it to either drop that limitation or
replace it with a short note pointing readers to the multi-agent guide and the
`sync-check`/`sync-apply` workflow.
Comment on lines
+431
to
+432
| result = expire_pending( | ||
| _store(), apply=apply, expired_by=EXPIRE_ACTOR, days=days, |
There was a problem hiding this comment.
Preserve the actual caller when expiring proposals.
kb_expire() hard-codes EXPIRE_ACTOR here, so the transport drops VOUCH_AGENT before the lifecycle layer sees the request. That makes operator-triggered expiry indistinguishable from automatic expiry in any expired_by or audit fields produced downstream.
Suggested fix
result = expire_pending(
- _store(), apply=apply, expired_by=EXPIRE_ACTOR, days=days,
+ _store(), apply=apply, expired_by=_agent(), days=days,
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| result = expire_pending( | |
| _store(), apply=apply, expired_by=EXPIRE_ACTOR, days=days, | |
| result = expire_pending( | |
| _store(), apply=apply, expired_by=_agent(), days=days, |
🤖 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 `@src/vouch/server.py` around lines 431 - 432, kb_expire() currently calls
expire_pending(..., expired_by=EXPIRE_ACTOR) which overwrites the
transport-supplied caller (e.g., VOUCH_AGENT) so downstream lifecycle/audit
fields lose operator identity; change kb_expire() to pass the actual incoming
caller/actor into expire_pending via the expired_by parameter (use the actor
value supplied to kb_expire() or the transport context) instead of EXPIRE_ACTOR
so the original caller is preserved through expire_pending and lifecycle/audit.
Comment on lines
370
to
399
| def update_claim(self, claim: Claim) -> Claim: | ||
| if not self._claim_path(claim.id).exists(): | ||
| raise ArtifactNotFoundError(f"claim {claim.id}") | ||
| # Re-validate the in-memory Claim before persisting so model | ||
| # invariants (e.g. evidence must be non-empty — see #81) hold | ||
| # even when a caller mutated fields in place after get_claim(). | ||
| # The Claim model's field validators only run at construction | ||
| # time; mutation alone bypasses them unless we round-trip. | ||
| Claim.model_validate(claim.model_dump(mode="json")) | ||
| self._claim_path(claim.id).write_text(_yaml_dump(claim.model_dump(mode="json"))) | ||
| self._embed_and_store(kind="claim", id=claim.id, text=claim.text) | ||
| # Keep the FTS5 row in sync with the on-disk claim so lifecycle | ||
| # mutations (archive, supersede, contradict, confirm) are reflected | ||
| # in retrieval immediately. Without this, claims_fts.status stays | ||
| # frozen at first-index time and retracted claims keep matching | ||
| # kb.search / kb.context. | ||
| from . import index_db as _index_db | ||
|
|
||
| try: | ||
| with _index_db.open_db(self.kb_dir) as conn: | ||
| _index_db.index_claim( | ||
| conn, id=claim.id, text=claim.text, | ||
| type=claim.type.value, status=claim.status.value, | ||
| tags=list(claim.tags), | ||
| ) | ||
| except sqlite3.Error as e: | ||
| _embed_log.warning( | ||
| "claim %s: FTS5 reindex skipped on update (%s)", claim.id, e, | ||
| ) | ||
| return claim |
There was a problem hiding this comment.
Keep update_claim() consistent with put_claim()'s citation existence checks.
This now re-validates the model shape, but it still skips the source/evidence existence loop that put_claim() runs. A caller can load a valid claim, mutate claim.evidence to an unknown id, and update_claim() will persist a broken claim that later fails citation resolution.
Suggested fix
def update_claim(self, claim: Claim) -> Claim:
if not self._claim_path(claim.id).exists():
raise ArtifactNotFoundError(f"claim {claim.id}")
@@
- Claim.model_validate(claim.model_dump(mode="json"))
- self._claim_path(claim.id).write_text(_yaml_dump(claim.model_dump(mode="json")))
- self._embed_and_store(kind="claim", id=claim.id, text=claim.text)
+ validated = Claim.model_validate(claim.model_dump(mode="json"))
+ for cid_or_sid in validated.evidence:
+ if (self._source_dir(cid_or_sid) / "meta.yaml").exists():
+ continue
+ if self._evidence_path(cid_or_sid).exists():
+ continue
+ raise ValueError(
+ f"claim {validated.id} cites unknown source/evidence {cid_or_sid}"
+ )
+ self._claim_path(validated.id).write_text(
+ _yaml_dump(validated.model_dump(mode="json"))
+ )
+ self._embed_and_store(kind="claim", id=validated.id, text=validated.text)
@@
_index_db.index_claim(
- conn, id=claim.id, text=claim.text,
- type=claim.type.value, status=claim.status.value,
- tags=list(claim.tags),
+ conn, id=validated.id, text=validated.text,
+ type=validated.type.value, status=validated.status.value,
+ tags=list(validated.tags),
)🤖 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 `@src/vouch/storage.py` around lines 370 - 399, update_claim currently
revalidates Claim shape but omits the citation/source existence checks that
put_claim performs, so mutated claim.evidence can reference unknown artifacts;
before writing and storing, replicate the same existence validation loop used in
put_claim (or extract it into a shared helper) to verify each evidence/source id
exists and raise ArtifactNotFoundError for any missing id, then proceed with
persistence, _embed_and_store and FTS5 reindexing as before.
Comment on lines
+148
to
+154
| def _load_bundle_source(source_path: Path) -> _SyncSource: | ||
| with tarfile.open(source_path, "r:gz") as tar: | ||
| try: | ||
| member = tar.getmember(bundle.MANIFEST_NAME) | ||
| except KeyError as exc: | ||
| raise RuntimeError("bundle missing manifest.json") from exc | ||
| manifest = json.loads(tar.extractfile(member).read().decode()) # type: ignore[union-attr] |
There was a problem hiding this comment.
Translate invalid bundle files into a sync error.
Any regular file reaches _load_bundle_source(), but tarfile.open() can raise tarfile.ReadError here. The CLI only normalizes RuntimeError / ValueError, so vouch sync-check some.txt currently risks a traceback instead of a one-line user error.
Suggested fix
def _load_bundle_source(source_path: Path) -> _SyncSource:
- with tarfile.open(source_path, "r:gz") as tar:
+ try:
+ tar = tarfile.open(source_path, "r:gz")
+ except tarfile.TarError as exc:
+ raise RuntimeError(
+ f"sync source is not a readable vouch bundle: {source_path}"
+ ) from exc
+ with tar:
try:
member = tar.getmember(bundle.MANIFEST_NAME)
except KeyError as exc:
raise RuntimeError("bundle missing manifest.json") from exc📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def _load_bundle_source(source_path: Path) -> _SyncSource: | |
| with tarfile.open(source_path, "r:gz") as tar: | |
| try: | |
| member = tar.getmember(bundle.MANIFEST_NAME) | |
| except KeyError as exc: | |
| raise RuntimeError("bundle missing manifest.json") from exc | |
| manifest = json.loads(tar.extractfile(member).read().decode()) # type: ignore[union-attr] | |
| def _load_bundle_source(source_path: Path) -> _SyncSource: | |
| try: | |
| tar = tarfile.open(source_path, "r:gz") | |
| except tarfile.TarError as exc: | |
| raise RuntimeError( | |
| f"sync source is not a readable vouch bundle: {source_path}" | |
| ) from exc | |
| with tar: | |
| try: | |
| member = tar.getmember(bundle.MANIFEST_NAME) | |
| except KeyError as exc: | |
| raise RuntimeError("bundle missing manifest.json") from exc | |
| manifest = json.loads(tar.extractfile(member).read().decode()) # type: ignore[union-attr] |
🤖 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 `@src/vouch/sync.py` around lines 148 - 154, _in _load_bundle_source, wrap the
tarfile.open(...) call in a try/except that catches tarfile.ReadError and
re-raises a normalized error (e.g., raise RuntimeError(f"invalid bundle file:
{source_path}") from exc) so invalid/regular files produce the same user-facing
sync error the CLI expects; keep the existing manifest lookup and JSON parsing
logic intact and preserve the original exception as the __cause__ when
re-raising.
Comment on lines
+30
to
+56
| def test_status_no_color_by_default_in_pipe( | ||
| store: KBStore, monkeypatch: pytest.MonkeyPatch | ||
| ) -> None: | ||
| monkeypatch.delenv("FORCE_COLOR", raising=False) | ||
| monkeypatch.delenv("NO_COLOR", raising=False) | ||
| result = CliRunner().invoke(cli, ["status"]) | ||
| assert result.exit_code == 0, result.output | ||
| assert "\x1b[" not in result.output # no ANSI when not a TTY | ||
|
|
||
|
|
||
| def test_status_force_color_emits_ansi( | ||
| store: KBStore, monkeypatch: pytest.MonkeyPatch | ||
| ) -> None: | ||
| monkeypatch.setenv("FORCE_COLOR", "1") | ||
| monkeypatch.delenv("NO_COLOR", raising=False) | ||
| result = CliRunner().invoke(cli, ["status"]) | ||
| assert result.exit_code == 0, result.output | ||
| assert "\x1b[" in result.output # ANSI present when forced | ||
|
|
||
|
|
||
| def test_no_color_wins_over_force_color( | ||
| store: KBStore, monkeypatch: pytest.MonkeyPatch | ||
| ) -> None: | ||
| monkeypatch.setenv("FORCE_COLOR", "1") | ||
| monkeypatch.setenv("NO_COLOR", "1") | ||
| result = CliRunner().invoke(cli, ["status"]) | ||
| assert "\x1b[" not in result.output |
There was a problem hiding this comment.
Ensure Click preserves ANSI in CliRunner output
CliRunner().invoke() defaults to color=False, so captured result.output can strip ANSI escapes—making the "\x1b[" assertions not reliably exercise your FORCE_COLOR/NO_COLOR precedence. Pass color=True in these status invocations.
Suggested change
- result = CliRunner().invoke(cli, ["status"])
+ result = CliRunner().invoke(cli, ["status"], color=True)
@@
- result = CliRunner().invoke(cli, ["status"])
+ result = CliRunner().invoke(cli, ["status"], color=True)
@@
- result = CliRunner().invoke(cli, ["status"])
+ result = CliRunner().invoke(cli, ["status"], color=True)🤖 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_cli_output.py` around lines 30 - 56, The tests
test_status_no_color_by_default_in_pipe, test_status_force_color_emits_ansi, and
test_no_color_wins_over_force_color call CliRunner().invoke(cli, ["status"])
which defaults to color=False and can strip ANSI sequences; update each
invocation to pass color=True (i.e., CliRunner().invoke(cli, ["status"],
color=True) ) so the result.output preserves ANSI escape sequences and the
"\x1b[" assertions reliably test FORCE_COLOR/NO_COLOR precedence.
Comment on lines
+78
to
+82
| def test_init_unknown_template_clean_error(tmp_path: Path) -> None: | ||
| res = CliRunner().invoke(cli, ["init", "--path", str(tmp_path), "--template", "bogus"]) | ||
| assert res.exit_code != 0 | ||
| assert "Traceback" not in res.output | ||
| assert "unknown template" in res.output.lower() |
There was a problem hiding this comment.
Assert the available-template list in the CLI error.
The design contract here is stronger than "unknown template" alone: the error is supposed to tell the user which template names are valid. This test won't catch a regression that drops that guidance.
Suggested assertion
def test_init_unknown_template_clean_error(tmp_path: Path) -> None:
res = CliRunner().invoke(cli, ["init", "--path", str(tmp_path), "--template", "bogus"])
assert res.exit_code != 0
assert "Traceback" not in res.output
assert "unknown template" in res.output.lower()
+ assert "gittensor" in res.output
+ assert "starter" in res.output🤖 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_templates.py` around lines 78 - 82, Update
test_init_unknown_template_clean_error to assert the CLI error includes the list
of valid template names in addition to the current checks: after invoking cli in
that test, import or call the source of truth for templates (e.g.
AVAILABLE_TEMPLATES constant or get_available_template_names() used by cli) and
assert that each expected template name (or the joined list string) appears in
res.output (case-insensitive) so the failure message shows the
available-template guidance.
Collaborator
ReviewSummary: This PR fixes the artifact-id path-traversal vulnerability described in #149 and bundles a large set of unrelated features (vouch fsck, vouch expire, vouch review, vouch diff, vouch sync-check/apply, batch approve, structured JSON logging, retrieval backend config, colorised CLI output). The security fix is solid and exactly matches the two-layer approach the issue specified. The bundled features are well-tested but out of scope for a targeted security patch. What works
Suggestions
Verdictapprove — The core security fix (model-layer |
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
10 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.
What changed
Adds a shared
_validate_artifact_idhelper insrc/vouch/models.pyand wires it as anid@field_validatoronClaim,Page,Entity,Relation,Evidence,Session, andProposal— rejecting empty/whitespace ids, path separators (/,\),.., NUL bytes, ASCII control characters, and leading..Sourcekeeps its stricter hex-sha256 validator (a content-addressed superset of this rule). On the storage layer, addsKBStore._safe_artifact_path(sub, obj_id, suffix)insrc/vouch/storage.pyas belt-and-suspenders: every_<X>_pathhelper (_yaml,_claim_path,_page_path,_source_dir,_entity_path,_relation_path,_evidence_path,_session_path) now resolves the constructed path and verifies it stays inside its artifact subdirectory.Fixes #149
Why
KBStore._yaml(sub, obj_id)turned the artifactidfield straight into a filesystem path (kb_dir/<sub>/{id}.{yaml,md}) with no containment check, and onlySource.idwas schema-validated. So a bareClaim(id="../escape")plusput_claimwrites a YAML one level abovekb_dir/claims/; with enough..segments the write lands outside the project tree entirely. Three paths reach this:Claim(id="../escape", ...)is accepted by Pydantic, thenput_claim's only existence check is onclaim.evidence, never onclaim.id.bundle._safe_member_path(CVE-2007-4559 / Path Traversal in Bundle Import — Arbitrary File Write (Critical) #9) sanitises tar member file paths, but the in-YAMLidfield is independent and was never gated._validate_contentrunsClaim.model_validate(...), which (before this change) accepted any id string. A manifest-consistent bundle could ship a poisoned id at a safe tar path; subsequentupdate_claim/ lifecycle ops would resolve the in-memory id straight to an off-tree write target.kb.archive/kb.supersede/kb.contradict(MCP and JSONL surfaces) accept aclaim_idand callstore.get_claim(claim_id)→store.update_claim(claim). An untrusted id propagates into both the read and the write side.Same structural shape as #81 (claim-citation validator), #123 (graph integrity), and #125 (source content-addressing): an invariant the project already enforces at one boundary (here:
bundle._unsafe_name_reasonfor tar member names) extended to the parallel surface (the in-YAML id field). The fix is the smallest one that closes all three reach paths simultaneously.What might break
Nothing on-disk, schema, bundle-format, MCP/JSONL surface, or audit-log shape — strictly additive validation. Honest slug-style ids (what
proposals._slugifyproduces, e.g.auth-uses-jwt) are unaffected; only data the documented containment contract already says shouldn't exist starts being rejected at write time.Claim/Page/Entity/Relation/Evidence/Session/Proposalconstruction with a path-unsafe id now raisespydantic.ValidationError(previously silent).store.put_<X>(<model>(...))paths inherit the model-layer rejection.bundle.import_check/import_applyandsync.sync_check/sync_applyreject any in-YAML id that violates the new validator via_validate_content's existing pydantic path — surfaced asschema validation failed: …path separators…exactly the way bug: Claim model has no min-evidence validator — uncited claims land via bundle import, put_claim, update_claim #81 / fix(models): require Claim.evidence to be non-empty at the model layer #82 surface uncited claims.invalid_claimfindings viavouch lint(the per-file resilience added in the fix(models): require Claim.evidence to be non-empty at the model layer #82 review) rather than crashing — mirrors that migration path exactly.VEP
Not required. No change to the object model fields,
kb.*method surface, on-disk layout, bundle format, or audit-log shape — this only tightens write-time validation to match a containment contractbundle._unsafe_name_reasonandread_under_rootalready document.Tests
make checkpasses locally —ruff check src testsclean,mypy srcreportsSuccess: no issues found in 32 source files, fullpytestgreen on Windows / Python 3.13 except the pre-existingtests/test_jsonl_server.py::test_read_under_root_rejects_symlink_at_resolved_pathwhich needs symlink-create privilege (unrelated; same env-only skip noted in fix(models): require Claim.evidence to be non-empty at the model layer #82 / fix: reject dangling Relation/Page references at every write path #124 / fix: enforce Source content-addressing on bundle/sync import #126).New / changed behaviour has tests — 13 new regressions:
tests/test_storage.py: per-model traversal-id rejection (Claim,Page,Entity,Relation,Evidence), NUL-byte rejection, leading-dot rejection, empty/whitespace rejection,put_claimdoesn't write off-tree,_<X>_pathstorage-layer guard for every artifact kind,update_claimrejection on in-place mutation to a traversal id.tests/test_bundle.py: bundle whose claim YAML hasid: "../escape"is rejected byimport_check(schema validation failed: …path separators…) andimport_applyraises; no off-tree file appears.tests/test_health.py: legacy KB with a traversal-id claim YAML on disk surfaces as aninvalid_claimfinding viavouch lintrather than crashing with a barepydantic.ValidationError— mirrorstest_lint_surfaces_legacy_uncited_claim_yaml_without_crashingfor the bug: Claim model has no min-evidence validator — uncited claims land via bundle import, put_claim, update_claim #81 / fix(models): require Claim.evidence to be non-empty at the model layer #82 migration.CHANGELOG.mdupdated under## [Unreleased]— add before merge (suggested entry):Summary by CodeRabbit
Release Notes
New Features
vouch fsckfor deeper read-only consistency checksvouch expireto remove stale pending proposalsvouch init --templatewith Gittensor starter template optionvouch reviewinteractive workflow for proposal approval/rejectionvouch sync-check/vouch sync-applyfor distributed knowledge base synchronizationvouch diffcommand to compare artifact revisionsvouch approvenow accepts multiple IDs with--keep-goingmode--jsonoutput forlint,search,pending, anddiffcommandsVOUCH_LOG_FORMAT=jsonBug Fixes
Documentation