fix: contain source verification file reads#424
Open
Steve-too wants to merge 1 commit into
Open
Conversation
📝 WalkthroughWalkthrough
ChangesContainment fix for verify_source
KB runtime and storage expansion
Vouch metadata and documentation artifacts
Desktop app and spec cleanup
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as vouch source verify
participant verify as verify_source
participant store as KBStore
CLI->>verify: verify_source(store, source)
verify->>store: read_under_root(source.locator)
alt path is contained and readable
store-->>verify: external_body
verify->>verify: hash external_body, compare to source.id
verify-->>CLI: external_status = match/drift
else OSError or ValueError raised
store-->>verify: exception
verify-->>CLI: external_status = missing, note = unreadable
end
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
tests
Steve-too
force-pushed
the
fix/verify-source-containment
branch
from
0a74dd5 to
4c88920
Compare
Author
|
Retargeted this PR to |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/vouch/storage.py (2)
530-541: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMirror
put_pagereference validation inupdate_page.
put_pagerejects dangling claims/entities/sources, butupdate_pagecan overwrite an existing page with invalid references and bypass that invariant.Proposed fix
def update_page(self, page: Page) -> Page: """Overwrite an existing page on disk. Used by the vault-edit approve path. @@ """ if not self._page_path(page.id).exists(): raise ArtifactNotFoundError(f"page {page.id}") + for cid in page.claims: + if not self._claim_path(cid).exists(): + raise ValueError(f"page {page.id} references unknown claim {cid}") + for eid in page.entities: + if not self._entity_path(eid).exists(): + raise ValueError(f"page {page.id} references unknown entity {eid}") + for sid in page.sources: + if not (self._source_dir(sid) / "meta.yaml").exists(): + raise ValueError(f"page {page.id} references unknown source {sid}") self._page_path(page.id).write_text( _serialize_page(page), encoding="utf-8" )🤖 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 530 - 541, `update_page` currently overwrites an existing page without enforcing the same reference integrity checks as `put_page`, so invalid claims/entities/sources can slip through. Update `update_page` in `Storage` to validate the page’s references before writing, using the same dangling-reference validation path already applied by `put_page`, and only proceed with the write once those checks pass. Keep the existing existence check and the index refresh behavior intact.
215-238: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftClose the symlink race in
read_under_root, and keep page updates on the same validation path as creates.
os.open(..., O_NOFOLLOW)only blocks a symlink in the final path component; a symlinked parent directory can still redirect the read outsideself.root. Re-open via anopenat-style component walk or recheck the opened fd’s real target underself.root.update_page()overwrites an existing page without the claim/entity/source reference checks thatput_page()performs, so edits can persist dangling references.🤖 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 215 - 238, `read_under_root` still has a symlink escape via parent directories, and `update_page` bypasses the same reference validation used by `put_page`. Harden `read_under_root` by resolving or opening the path component-by-component and verifying the final fd still points under `self.root`, not just the resolved path, then keep the file-read logic on that validated fd. For `update_page`, route edits through the same claim/entity/source reference checks as `put_page` (or factor them into a shared validation helper) so page updates cannot persist dangling references.src/vouch/context.py (1)
354-358: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBuild
explainfrom the filtered context items.Lines 269-274 intentionally skip missing/retracted claims, but
explainstill exposes the rawhitsids and scores. Use the accepteditemsso explain output cannot leak filtered artifacts.Proposed fix
if explain: result["explain"] = [ - {"kind": k, "id": i, "score": sc, "backend": hits[0][4] if hits else "none"} - for k, i, _sn, sc, _be in hits + {"kind": it.type, "id": it.id, "score": it.score, "backend": it.backend} + for it in items ]🤖 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 354 - 358, The explain payload in the context-building flow is still derived from raw hits instead of the accepted filtered items, so it can expose skipped artifacts. Update the explain construction in the same context method that builds result["explain"] to iterate over the filtered items used after missing/retracted claims are removed, and source id/score/backend from those accepted entries rather than hits. Keep the explain shape the same, but ensure it only reflects items that survived filtering.src/vouch/jsonl_server.py (1)
742-789: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReview the CLI mirror for
kb.eval_embeddings
The MCP surface, capabilities list, and JSONL handler map include these new tools; the remaining gap is whether the CLI exposes the same embedding-evaluation entrypoint, since the CLI only shows related commands (eval_group,eval_embedding,eval_recall).🤖 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/jsonl_server.py` around lines 742 - 789, The JSONL handler map already exposes kb.eval_embeddings, but the CLI mirror still appears to be missing the matching entrypoint. Update the CLI command registration around the eval-related commands so the same embedding-evaluation capability is available through the CLI, and make sure the new command is wired consistently with the existing eval_group, eval_embedding, and eval_recall handlers.Source: Coding guidelines
🤖 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 @.github/workflows/security-audit.yml:
- Line 27: The security-audit workflow currently uses actions/checkout@v4
without disabling credential persistence, which leaves the GitHub token
available in the repo config for the pip install -e . step. Update the checkout
configuration in the security-audit job to turn off persisted credentials so the
PR audit job does not retain token access after checkout. Use the
actions/checkout step as the target for this change.
In
@.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md:
- Around line 36-38: The session note currently lists absolute machine-specific
paths that leak local environment details; update the non-development files list
to remove or generalize those `/home/a/...` and `/tmp/...` entries. Locate the
paths in the session note content and replace them with portable,
checkout-agnostic references so the note remains valid across environments.
In `@CONTRIBUTING.md`:
- Around line 128-130: The proof requirement in CONTRIBUTING.md is too narrow
because it ties all user-facing changes to vouch-ui screenshots, even for
CLI-only or non-webapp work. Update the guidance around the “changes that affect
the webapp, CLI output, or user-facing behavior” section to clearly allow proof
appropriate to the change type, such as CLI output captures or other relevant
artifacts, and keep vouch-ui screenshots only when the webapp is actually
impacted.
---
Outside diff comments:
In `@src/vouch/context.py`:
- Around line 354-358: The explain payload in the context-building flow is still
derived from raw hits instead of the accepted filtered items, so it can expose
skipped artifacts. Update the explain construction in the same context method
that builds result["explain"] to iterate over the filtered items used after
missing/retracted claims are removed, and source id/score/backend from those
accepted entries rather than hits. Keep the explain shape the same, but ensure
it only reflects items that survived filtering.
In `@src/vouch/jsonl_server.py`:
- Around line 742-789: The JSONL handler map already exposes kb.eval_embeddings,
but the CLI mirror still appears to be missing the matching entrypoint. Update
the CLI command registration around the eval-related commands so the same
embedding-evaluation capability is available through the CLI, and make sure the
new command is wired consistently with the existing eval_group, eval_embedding,
and eval_recall handlers.
In `@src/vouch/storage.py`:
- Around line 530-541: `update_page` currently overwrites an existing page
without enforcing the same reference integrity checks as `put_page`, so invalid
claims/entities/sources can slip through. Update `update_page` in `Storage` to
validate the page’s references before writing, using the same dangling-reference
validation path already applied by `put_page`, and only proceed with the write
once those checks pass. Keep the existing existence check and the index refresh
behavior intact.
- Around line 215-238: `read_under_root` still has a symlink escape via parent
directories, and `update_page` bypasses the same reference validation used by
`put_page`. Harden `read_under_root` by resolving or opening the path
component-by-component and verifying the final fd still points under
`self.root`, not just the resolved path, then keep the file-read logic on that
validated fd. For `update_page`, route edits through the same
claim/entity/source reference checks as `put_page` (or factor them into a shared
validation helper) so page updates cannot persist dangling references.
🪄 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: 29feaf90-875e-4fd5-b322-88b843d6376c
⛔ Files ignored due to path filters (6)
desktop/docs/screenshots/browse.pngis excluded by!**/*.pngdesktop/docs/screenshots/dashboard.pngis excluded by!**/*.pngdesktop/docs/screenshots/dual-solve.pngis excluded by!**/*.pngdesktop/docs/screenshots/review.pngis excluded by!**/*.pngdesktop/package-lock.jsonis excluded by!**/package-lock.jsondocs/banner.svgis excluded by!**/*.svg
📒 Files selected for processing (114)
.github/workflows/security-audit.yml.pre-commit-config.yaml.vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl.vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl.vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl.vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl.vouch/claims/vouch-starter-reviewed-knowledge.yaml.vouch/decided/20260707-092531-154cb163.yaml.vouch/decided/20260707-093005-05345f47.yaml.vouch/decided/20260707-093125-4d6972f9.yaml.vouch/pages/edit-in-obsidian.md.vouch/pages/reviewed-knowledge-store.md.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md.vouch/schema_version.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yamlCONTRIBUTING.mdREADME.mddesktop/.gitignoredesktop/CHANGELOG.mddesktop/LICENSEdesktop/README.mddesktop/docs/architecture.mddesktop/docs/reports/2026-06-26.mddesktop/electron-builder.ymldesktop/electron.vite.config.tsdesktop/package.jsondesktop/resources/vouch/.gitkeepdesktop/scripts/dev-with-vouch.tsdesktop/scripts/gen-methods.tsdesktop/src/catalog/backend.jsondesktop/src/catalog/methods.jsondesktop/src/main/http-client.tsdesktop/src/main/index.tsdesktop/src/main/ipc.tsdesktop/src/main/jsonl-client.tsdesktop/src/main/kb-store.tsdesktop/src/main/supervisor.tsdesktop/src/main/tray.tsdesktop/src/main/types.tsdesktop/src/main/vouch-locator.tsdesktop/src/preload/index.tsdesktop/src/renderer/index.htmldesktop/src/renderer/src/App.tsxdesktop/src/renderer/src/app.cssdesktop/src/renderer/src/components/Diff.tsxdesktop/src/renderer/src/components/Drawer.tsxdesktop/src/renderer/src/components/EmptyState.tsxdesktop/src/renderer/src/components/MethodCard.tsxdesktop/src/renderer/src/components/MethodForm.tsxdesktop/src/renderer/src/components/Placeholder.tsxdesktop/src/renderer/src/components/Rail.tsxdesktop/src/renderer/src/components/StatusBar.tsxdesktop/src/renderer/src/components/Topbar.tsxdesktop/src/renderer/src/components/controls/index.tsxdesktop/src/renderer/src/components/results/Cards.tsxdesktop/src/renderer/src/components/results/JsonTree.tsxdesktop/src/renderer/src/components/results/Renderers.tsxdesktop/src/renderer/src/components/results/ResultView.tsxdesktop/src/renderer/src/components/results/atoms.tsxdesktop/src/renderer/src/global.d.tsdesktop/src/renderer/src/lib/VouchContext.tsxdesktop/src/renderer/src/lib/client.tsdesktop/src/renderer/src/lib/format.tsdesktop/src/renderer/src/lib/useOnOpen.tsxdesktop/src/renderer/src/lib/useVouchEvents.tsdesktop/src/renderer/src/main.tsxdesktop/src/renderer/src/views/Dashboard.tsxdesktop/src/renderer/src/views/DualSolve.tsxdesktop/src/renderer/src/views/GenericView.tsxdesktop/src/renderer/src/views/Review.tsxdesktop/src/renderer/src/views/blurbs.tsdesktop/src/renderer/src/views/registry.tsxdesktop/src/shared/ipc.tsdesktop/src/shared/methods.gen.tsdesktop/src/shared/methods.types.tsdesktop/test/catalog.test.tsdesktop/test/controls.test.tsxdesktop/test/gen-methods.test.tsdesktop/test/http-client.test.tsdesktop/test/jsonl-client.test.tsdesktop/test/method-card.test.tsxdesktop/test/method-form.test.tsxdesktop/test/method-gate.test.tsdesktop/test/smoke-jsonl.tsdesktop/test/vouch-locator.test.tsdesktop/tsconfig.jsondesktop/tsconfig.node.jsondesktop/tsconfig.scripts.jsondesktop/tsconfig.web.jsondesktop/vitest.config.tsdocs/superpowers/specs/2026-07-06-review-ui-multi-kb-design.mdmigrations/README.mdspec/2026-05-21/README.mdspec/2026-05-21/SPEC.mdspec/2026-05-21/audit-vocabulary.mdspec/2026-05-21/methods.mdspec/2026-05-21/retrieval.mdspec/2026-05-21/review-gate.mdspec/2026-05-21/transports.mdspec/README.mdspec/audit-vocabulary.mdspec/methods.mdspec/retrieval.mdspec/review-gate.mdspec/transports.mdsrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/storage.pysrc/vouch/verify.pytests/test_verify.py
💤 Files with no reviewable changes (88)
- desktop/docs/reports/2026-06-26.md
- desktop/CHANGELOG.md
- desktop/src/renderer/src/components/Placeholder.tsx
- migrations/README.md
- desktop/tsconfig.node.json
- desktop/LICENSE
- desktop/docs/architecture.md
- desktop/src/renderer/src/components/results/atoms.tsx
- desktop/src/main/types.ts
- .pre-commit-config.yaml
- desktop/electron.vite.config.ts
- desktop/.gitignore
- desktop/src/catalog/backend.json
- desktop/scripts/dev-with-vouch.ts
- desktop/src/renderer/src/components/Drawer.tsx
- desktop/src/catalog/methods.json
- desktop/src/shared/methods.types.ts
- desktop/test/jsonl-client.test.ts
- desktop/README.md
- desktop/src/renderer/src/components/EmptyState.tsx
- desktop/src/renderer/src/views/blurbs.ts
- spec/2026-05-21/transports.md
- spec/2026-05-21/audit-vocabulary.md
- desktop/src/renderer/src/lib/useVouchEvents.ts
- desktop/tsconfig.json
- spec/2026-05-21/retrieval.md
- desktop/scripts/gen-methods.ts
- desktop/test/smoke-jsonl.ts
- spec/methods.md
- desktop/src/main/index.ts
- spec/transports.md
- desktop/test/method-card.test.tsx
- desktop/src/renderer/src/components/MethodCard.tsx
- desktop/package.json
- desktop/src/renderer/src/components/results/JsonTree.tsx
- spec/review-gate.md
- desktop/test/http-client.test.ts
- desktop/src/renderer/src/components/Topbar.tsx
- spec/README.md
- desktop/test/catalog.test.ts
- desktop/src/renderer/index.html
- desktop/test/controls.test.tsx
- desktop/src/renderer/src/components/StatusBar.tsx
- desktop/src/renderer/src/global.d.ts
- spec/2026-05-21/SPEC.md
- desktop/src/renderer/src/lib/useOnOpen.tsx
- desktop/src/renderer/src/components/Rail.tsx
- desktop/src/main/kb-store.ts
- spec/2026-05-21/methods.md
- desktop/src/renderer/src/lib/client.ts
- desktop/src/renderer/src/views/Dashboard.tsx
- desktop/src/renderer/src/main.tsx
- spec/2026-05-21/review-gate.md
- spec/retrieval.md
- desktop/vitest.config.ts
- desktop/src/renderer/src/components/MethodForm.tsx
- desktop/src/renderer/src/views/registry.tsx
- desktop/test/method-gate.test.ts
- desktop/test/method-form.test.tsx
- desktop/src/renderer/src/views/DualSolve.tsx
- desktop/tsconfig.web.json
- desktop/src/main/http-client.ts
- desktop/src/renderer/src/components/results/ResultView.tsx
- desktop/tsconfig.scripts.json
- desktop/src/renderer/src/App.tsx
- desktop/src/renderer/src/components/controls/index.tsx
- desktop/src/shared/ipc.ts
- desktop/src/renderer/src/components/Diff.tsx
- desktop/src/renderer/src/lib/VouchContext.tsx
- spec/2026-05-21/README.md
- desktop/test/vouch-locator.test.ts
- desktop/src/renderer/src/views/GenericView.tsx
- desktop/src/renderer/src/lib/format.ts
- desktop/src/preload/index.ts
- desktop/src/main/tray.ts
- desktop/src/renderer/src/components/results/Renderers.tsx
- spec/audit-vocabulary.md
- desktop/src/main/vouch-locator.ts
- desktop/src/main/jsonl-client.ts
- desktop/src/shared/methods.gen.ts
- desktop/src/main/ipc.ts
- desktop/test/gen-methods.test.ts
- desktop/src/main/supervisor.ts
- src/vouch/verify.py
- desktop/src/renderer/src/app.css
- desktop/src/renderer/src/components/results/Cards.tsx
- desktop/electron-builder.yml
- desktop/src/renderer/src/views/Review.tsx
✅ Files skipped from review due to trivial changes (12)
- .vouch/schema_version
- .vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content
- .vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl
- .vouch/pages/reviewed-knowledge-store.md
- .vouch/decided/20260707-092531-154cb163.yaml
- .vouch/pages/edit-in-obsidian.md
- .vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yaml
- .vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl
- .vouch/decided/20260707-093005-05345f47.yaml
- README.md
- .vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl
- .vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/vouch/storage.py (2)
530-541: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMirror
put_pagereference validation inupdate_page.
put_pagerejects dangling claims/entities/sources, butupdate_pagecan overwrite an existing page with invalid references and bypass that invariant.Proposed fix
def update_page(self, page: Page) -> Page: """Overwrite an existing page on disk. Used by the vault-edit approve path. @@ """ if not self._page_path(page.id).exists(): raise ArtifactNotFoundError(f"page {page.id}") + for cid in page.claims: + if not self._claim_path(cid).exists(): + raise ValueError(f"page {page.id} references unknown claim {cid}") + for eid in page.entities: + if not self._entity_path(eid).exists(): + raise ValueError(f"page {page.id} references unknown entity {eid}") + for sid in page.sources: + if not (self._source_dir(sid) / "meta.yaml").exists(): + raise ValueError(f"page {page.id} references unknown source {sid}") self._page_path(page.id).write_text( _serialize_page(page), encoding="utf-8" )🤖 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 530 - 541, `update_page` currently overwrites an existing page without enforcing the same reference integrity checks as `put_page`, so invalid claims/entities/sources can slip through. Update `update_page` in `Storage` to validate the page’s references before writing, using the same dangling-reference validation path already applied by `put_page`, and only proceed with the write once those checks pass. Keep the existing existence check and the index refresh behavior intact.
215-238: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftClose the symlink race in
read_under_root, and keep page updates on the same validation path as creates.
os.open(..., O_NOFOLLOW)only blocks a symlink in the final path component; a symlinked parent directory can still redirect the read outsideself.root. Re-open via anopenat-style component walk or recheck the opened fd’s real target underself.root.update_page()overwrites an existing page without the claim/entity/source reference checks thatput_page()performs, so edits can persist dangling references.🤖 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 215 - 238, `read_under_root` still has a symlink escape via parent directories, and `update_page` bypasses the same reference validation used by `put_page`. Harden `read_under_root` by resolving or opening the path component-by-component and verifying the final fd still points under `self.root`, not just the resolved path, then keep the file-read logic on that validated fd. For `update_page`, route edits through the same claim/entity/source reference checks as `put_page` (or factor them into a shared validation helper) so page updates cannot persist dangling references.src/vouch/context.py (1)
354-358: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winBuild
explainfrom the filtered context items.Lines 269-274 intentionally skip missing/retracted claims, but
explainstill exposes the rawhitsids and scores. Use the accepteditemsso explain output cannot leak filtered artifacts.Proposed fix
if explain: result["explain"] = [ - {"kind": k, "id": i, "score": sc, "backend": hits[0][4] if hits else "none"} - for k, i, _sn, sc, _be in hits + {"kind": it.type, "id": it.id, "score": it.score, "backend": it.backend} + for it in items ]🤖 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 354 - 358, The explain payload in the context-building flow is still derived from raw hits instead of the accepted filtered items, so it can expose skipped artifacts. Update the explain construction in the same context method that builds result["explain"] to iterate over the filtered items used after missing/retracted claims are removed, and source id/score/backend from those accepted entries rather than hits. Keep the explain shape the same, but ensure it only reflects items that survived filtering.src/vouch/jsonl_server.py (1)
742-789: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReview the CLI mirror for
kb.eval_embeddings
The MCP surface, capabilities list, and JSONL handler map include these new tools; the remaining gap is whether the CLI exposes the same embedding-evaluation entrypoint, since the CLI only shows related commands (eval_group,eval_embedding,eval_recall).🤖 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/jsonl_server.py` around lines 742 - 789, The JSONL handler map already exposes kb.eval_embeddings, but the CLI mirror still appears to be missing the matching entrypoint. Update the CLI command registration around the eval-related commands so the same embedding-evaluation capability is available through the CLI, and make sure the new command is wired consistently with the existing eval_group, eval_embedding, and eval_recall handlers.Source: Coding guidelines
🤖 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 @.github/workflows/security-audit.yml:
- Line 27: The security-audit workflow currently uses actions/checkout@v4
without disabling credential persistence, which leaves the GitHub token
available in the repo config for the pip install -e . step. Update the checkout
configuration in the security-audit job to turn off persisted credentials so the
PR audit job does not retain token access after checkout. Use the
actions/checkout step as the target for this change.
In
@.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md:
- Around line 36-38: The session note currently lists absolute machine-specific
paths that leak local environment details; update the non-development files list
to remove or generalize those `/home/a/...` and `/tmp/...` entries. Locate the
paths in the session note content and replace them with portable,
checkout-agnostic references so the note remains valid across environments.
In `@CONTRIBUTING.md`:
- Around line 128-130: The proof requirement in CONTRIBUTING.md is too narrow
because it ties all user-facing changes to vouch-ui screenshots, even for
CLI-only or non-webapp work. Update the guidance around the “changes that affect
the webapp, CLI output, or user-facing behavior” section to clearly allow proof
appropriate to the change type, such as CLI output captures or other relevant
artifacts, and keep vouch-ui screenshots only when the webapp is actually
impacted.
---
Outside diff comments:
In `@src/vouch/context.py`:
- Around line 354-358: The explain payload in the context-building flow is still
derived from raw hits instead of the accepted filtered items, so it can expose
skipped artifacts. Update the explain construction in the same context method
that builds result["explain"] to iterate over the filtered items used after
missing/retracted claims are removed, and source id/score/backend from those
accepted entries rather than hits. Keep the explain shape the same, but ensure
it only reflects items that survived filtering.
In `@src/vouch/jsonl_server.py`:
- Around line 742-789: The JSONL handler map already exposes kb.eval_embeddings,
but the CLI mirror still appears to be missing the matching entrypoint. Update
the CLI command registration around the eval-related commands so the same
embedding-evaluation capability is available through the CLI, and make sure the
new command is wired consistently with the existing eval_group, eval_embedding,
and eval_recall handlers.
In `@src/vouch/storage.py`:
- Around line 530-541: `update_page` currently overwrites an existing page
without enforcing the same reference integrity checks as `put_page`, so invalid
claims/entities/sources can slip through. Update `update_page` in `Storage` to
validate the page’s references before writing, using the same dangling-reference
validation path already applied by `put_page`, and only proceed with the write
once those checks pass. Keep the existing existence check and the index refresh
behavior intact.
- Around line 215-238: `read_under_root` still has a symlink escape via parent
directories, and `update_page` bypasses the same reference validation used by
`put_page`. Harden `read_under_root` by resolving or opening the path
component-by-component and verifying the final fd still points under
`self.root`, not just the resolved path, then keep the file-read logic on that
validated fd. For `update_page`, route edits through the same
claim/entity/source reference checks as `put_page` (or factor them into a shared
validation helper) so page updates cannot persist dangling references.
🪄 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: 29feaf90-875e-4fd5-b322-88b843d6376c
⛔ Files ignored due to path filters (6)
desktop/docs/screenshots/browse.pngis excluded by!**/*.pngdesktop/docs/screenshots/dashboard.pngis excluded by!**/*.pngdesktop/docs/screenshots/dual-solve.pngis excluded by!**/*.pngdesktop/docs/screenshots/review.pngis excluded by!**/*.pngdesktop/package-lock.jsonis excluded by!**/package-lock.jsondocs/banner.svgis excluded by!**/*.svg
📒 Files selected for processing (114)
.github/workflows/security-audit.yml.pre-commit-config.yaml.vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl.vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl.vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl.vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl.vouch/claims/vouch-starter-reviewed-knowledge.yaml.vouch/decided/20260707-092531-154cb163.yaml.vouch/decided/20260707-093005-05345f47.yaml.vouch/decided/20260707-093125-4d6972f9.yaml.vouch/pages/edit-in-obsidian.md.vouch/pages/reviewed-knowledge-store.md.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md.vouch/schema_version.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content.vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yamlCONTRIBUTING.mdREADME.mddesktop/.gitignoredesktop/CHANGELOG.mddesktop/LICENSEdesktop/README.mddesktop/docs/architecture.mddesktop/docs/reports/2026-06-26.mddesktop/electron-builder.ymldesktop/electron.vite.config.tsdesktop/package.jsondesktop/resources/vouch/.gitkeepdesktop/scripts/dev-with-vouch.tsdesktop/scripts/gen-methods.tsdesktop/src/catalog/backend.jsondesktop/src/catalog/methods.jsondesktop/src/main/http-client.tsdesktop/src/main/index.tsdesktop/src/main/ipc.tsdesktop/src/main/jsonl-client.tsdesktop/src/main/kb-store.tsdesktop/src/main/supervisor.tsdesktop/src/main/tray.tsdesktop/src/main/types.tsdesktop/src/main/vouch-locator.tsdesktop/src/preload/index.tsdesktop/src/renderer/index.htmldesktop/src/renderer/src/App.tsxdesktop/src/renderer/src/app.cssdesktop/src/renderer/src/components/Diff.tsxdesktop/src/renderer/src/components/Drawer.tsxdesktop/src/renderer/src/components/EmptyState.tsxdesktop/src/renderer/src/components/MethodCard.tsxdesktop/src/renderer/src/components/MethodForm.tsxdesktop/src/renderer/src/components/Placeholder.tsxdesktop/src/renderer/src/components/Rail.tsxdesktop/src/renderer/src/components/StatusBar.tsxdesktop/src/renderer/src/components/Topbar.tsxdesktop/src/renderer/src/components/controls/index.tsxdesktop/src/renderer/src/components/results/Cards.tsxdesktop/src/renderer/src/components/results/JsonTree.tsxdesktop/src/renderer/src/components/results/Renderers.tsxdesktop/src/renderer/src/components/results/ResultView.tsxdesktop/src/renderer/src/components/results/atoms.tsxdesktop/src/renderer/src/global.d.tsdesktop/src/renderer/src/lib/VouchContext.tsxdesktop/src/renderer/src/lib/client.tsdesktop/src/renderer/src/lib/format.tsdesktop/src/renderer/src/lib/useOnOpen.tsxdesktop/src/renderer/src/lib/useVouchEvents.tsdesktop/src/renderer/src/main.tsxdesktop/src/renderer/src/views/Dashboard.tsxdesktop/src/renderer/src/views/DualSolve.tsxdesktop/src/renderer/src/views/GenericView.tsxdesktop/src/renderer/src/views/Review.tsxdesktop/src/renderer/src/views/blurbs.tsdesktop/src/renderer/src/views/registry.tsxdesktop/src/shared/ipc.tsdesktop/src/shared/methods.gen.tsdesktop/src/shared/methods.types.tsdesktop/test/catalog.test.tsdesktop/test/controls.test.tsxdesktop/test/gen-methods.test.tsdesktop/test/http-client.test.tsdesktop/test/jsonl-client.test.tsdesktop/test/method-card.test.tsxdesktop/test/method-form.test.tsxdesktop/test/method-gate.test.tsdesktop/test/smoke-jsonl.tsdesktop/test/vouch-locator.test.tsdesktop/tsconfig.jsondesktop/tsconfig.node.jsondesktop/tsconfig.scripts.jsondesktop/tsconfig.web.jsondesktop/vitest.config.tsdocs/superpowers/specs/2026-07-06-review-ui-multi-kb-design.mdmigrations/README.mdspec/2026-05-21/README.mdspec/2026-05-21/SPEC.mdspec/2026-05-21/audit-vocabulary.mdspec/2026-05-21/methods.mdspec/2026-05-21/retrieval.mdspec/2026-05-21/review-gate.mdspec/2026-05-21/transports.mdspec/README.mdspec/audit-vocabulary.mdspec/methods.mdspec/retrieval.mdspec/review-gate.mdspec/transports.mdsrc/vouch/cli.pysrc/vouch/context.pysrc/vouch/jsonl_server.pysrc/vouch/server.pysrc/vouch/sessions.pysrc/vouch/storage.pysrc/vouch/verify.pytests/test_verify.py
💤 Files with no reviewable changes (88)
- desktop/docs/reports/2026-06-26.md
- desktop/CHANGELOG.md
- desktop/src/renderer/src/components/Placeholder.tsx
- migrations/README.md
- desktop/tsconfig.node.json
- desktop/LICENSE
- desktop/docs/architecture.md
- desktop/src/renderer/src/components/results/atoms.tsx
- desktop/src/main/types.ts
- .pre-commit-config.yaml
- desktop/electron.vite.config.ts
- desktop/.gitignore
- desktop/src/catalog/backend.json
- desktop/scripts/dev-with-vouch.ts
- desktop/src/renderer/src/components/Drawer.tsx
- desktop/src/catalog/methods.json
- desktop/src/shared/methods.types.ts
- desktop/test/jsonl-client.test.ts
- desktop/README.md
- desktop/src/renderer/src/components/EmptyState.tsx
- desktop/src/renderer/src/views/blurbs.ts
- spec/2026-05-21/transports.md
- spec/2026-05-21/audit-vocabulary.md
- desktop/src/renderer/src/lib/useVouchEvents.ts
- desktop/tsconfig.json
- spec/2026-05-21/retrieval.md
- desktop/scripts/gen-methods.ts
- desktop/test/smoke-jsonl.ts
- spec/methods.md
- desktop/src/main/index.ts
- spec/transports.md
- desktop/test/method-card.test.tsx
- desktop/src/renderer/src/components/MethodCard.tsx
- desktop/package.json
- desktop/src/renderer/src/components/results/JsonTree.tsx
- spec/review-gate.md
- desktop/test/http-client.test.ts
- desktop/src/renderer/src/components/Topbar.tsx
- spec/README.md
- desktop/test/catalog.test.ts
- desktop/src/renderer/index.html
- desktop/test/controls.test.tsx
- desktop/src/renderer/src/components/StatusBar.tsx
- desktop/src/renderer/src/global.d.ts
- spec/2026-05-21/SPEC.md
- desktop/src/renderer/src/lib/useOnOpen.tsx
- desktop/src/renderer/src/components/Rail.tsx
- desktop/src/main/kb-store.ts
- spec/2026-05-21/methods.md
- desktop/src/renderer/src/lib/client.ts
- desktop/src/renderer/src/views/Dashboard.tsx
- desktop/src/renderer/src/main.tsx
- spec/2026-05-21/review-gate.md
- spec/retrieval.md
- desktop/vitest.config.ts
- desktop/src/renderer/src/components/MethodForm.tsx
- desktop/src/renderer/src/views/registry.tsx
- desktop/test/method-gate.test.ts
- desktop/test/method-form.test.tsx
- desktop/src/renderer/src/views/DualSolve.tsx
- desktop/tsconfig.web.json
- desktop/src/main/http-client.ts
- desktop/src/renderer/src/components/results/ResultView.tsx
- desktop/tsconfig.scripts.json
- desktop/src/renderer/src/App.tsx
- desktop/src/renderer/src/components/controls/index.tsx
- desktop/src/shared/ipc.ts
- desktop/src/renderer/src/components/Diff.tsx
- desktop/src/renderer/src/lib/VouchContext.tsx
- spec/2026-05-21/README.md
- desktop/test/vouch-locator.test.ts
- desktop/src/renderer/src/views/GenericView.tsx
- desktop/src/renderer/src/lib/format.ts
- desktop/src/preload/index.ts
- desktop/src/main/tray.ts
- desktop/src/renderer/src/components/results/Renderers.tsx
- spec/audit-vocabulary.md
- desktop/src/main/vouch-locator.ts
- desktop/src/main/jsonl-client.ts
- desktop/src/shared/methods.gen.ts
- desktop/src/main/ipc.ts
- desktop/test/gen-methods.test.ts
- desktop/src/main/supervisor.ts
- src/vouch/verify.py
- desktop/src/renderer/src/app.css
- desktop/src/renderer/src/components/results/Cards.tsx
- desktop/electron-builder.yml
- desktop/src/renderer/src/views/Review.tsx
✅ Files skipped from review due to trivial changes (12)
- .vouch/schema_version
- .vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/content
- .vouch/captures/1218169c-de25-48f4-bd93-70ff77d2fda2.jsonl
- .vouch/pages/reviewed-knowledge-store.md
- .vouch/decided/20260707-092531-154cb163.yaml
- .vouch/pages/edit-in-obsidian.md
- .vouch/sources/be7aec64b0fc803a33cb3d610f67ae95e636877db20231ef72440a7cbe6b69d2/meta.yaml
- .vouch/captures/5720220c-b77c-4947-aeec-9d6ef91d70b8.jsonl
- .vouch/decided/20260707-093005-05345f47.yaml
- README.md
- .vouch/captures/ec4b8842-f167-467e-a33c-dc87911c9e0e.jsonl
- .vouch/captures/8a6df051-6c56-4d86-b0ed-66f6638ecfdc.jsonl
🛑 Comments failed to post (3)
.github/workflows/security-audit.yml (1)
27-27: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable credential persistence for this PR audit job.
This workflow checks out PR code and then runs
pip install -e .; leaving the GitHub token in the checkout config is unnecessary and increases token exposure risk.Proposed fix
- - uses: actions/checkout@v4 + - uses: actions/checkout@v4 + with: + persist-credentials: false📝 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.- uses: actions/checkout@v4 with: persist-credentials: false🧰 Tools
🪛 zizmor (1.26.1)
[warning] 27-27: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 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 @.github/workflows/security-audit.yml at line 27, The security-audit workflow currently uses actions/checkout@v4 without disabling credential persistence, which leaves the GitHub token available in the repo config for the pip install -e . step. Update the checkout configuration in the security-audit job to turn off persisted credentials so the PR audit job does not retain token access after checkout. Use the actions/checkout step as the target for this change.Source: Linters/SAST tools
.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md (1)
36-38: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Redact machine-specific paths from the session note.
These absolute
/home/a/...and/tmp/...entries leak local environment details and won’t mean anything in other checkouts.Proposed fix
-- /home/a/.claude/projects/-home-a-Dev-plind-junior-vouch/memory/MEMORY.md -- /home/a/.claude/projects/-home-a-Dev-plind-junior-vouch/memory/commit-scope-src-adapters.md -- /tmp/claude-1000/-home-a-Dev-plind-junior-vouch/e5ead202-24c2-4916-b59c-ddb4c123d63f/scratchpad/merge_hook.py📝 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.🤖 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 @.vouch/pages/session-what-are-the-non-development-related-files-and-folde.md around lines 36 - 38, The session note currently lists absolute machine-specific paths that leak local environment details; update the non-development files list to remove or generalize those `/home/a/...` and `/tmp/...` entries. Locate the paths in the session note content and replace them with portable, checkout-agnostic references so the note remains valid across environments.CONTRIBUTING.md (1)
128-130: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Broaden the proof requirement beyond the vouch-ui webapp.
This wording forces CLI-only or other non-webapp changes to provide webapp screenshots, which doesn’t match the change scope.
Proposed fix
-- For changes that affect the webapp, CLI output, or user-facing behavior: -- screenshots or screenrecording from the vouch-ui webapp showing the -- behavior working as intended. +- For changes that affect the webapp, CLI output, or user-facing behavior: +- screenshots, terminal captures, or screen recordings showing the +- relevant behavior working as intended.📝 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.- For changes that affect the webapp, CLI output, or user-facing behavior: screenshots, terminal captures, or screen recordings showing the relevant behavior working as intended.🧰 Tools
🪛 LanguageTool
[grammar] ~128-~128: Ensure spelling is correct
Context: ... commits. - For changes that affect the webapp, CLI output, or user-facing behavior: ...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[grammar] ~129-~129: Ensure spelling is correct
Context: ...ts or screenrecording from the vouch-ui webapp showing the behavior working as inten...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@CONTRIBUTING.md` around lines 128 - 130, The proof requirement in CONTRIBUTING.md is too narrow because it ties all user-facing changes to vouch-ui screenshots, even for CLI-only or non-webapp work. Update the guidance around the “changes that affect the webapp, CLI output, or user-facing behavior” section to clearly allow proof appropriate to the change type, such as CLI output captures or other relevant artifacts, and keep vouch-ui screenshots only when the webapp is actually impacted.
Author
|
Note on the latest CodeRabbit out-of-scope warning: this appears to be an artifact of retargeting the PR from GitHub's current PR file list is still limited to the intended two files:
Verified with: gh pr view 424 --repo vouchdev/vouch --json files,baseRefName,changedFiles \
--jq '{base: .baseRefName, changed: .changedFiles, files: [.files[].path]}'Result: |
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
1 participant
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.
Closes #157.
Summary
KBStore.read_under_rootinstead of directPath(locator).read_bytes()missingverification resultsTests
PYTHONPATH=src python3 -m pytest tests/test_verify.py -qSummary by CodeRabbit
Bug Fixes
Tests
Documentation