fix(security): audit security findings — traversal, secrets, supply chain#497
Merged
Conversation
… liability) src/core/plugin-installer.ts built a `git clone` shell string from caller input (execSync interpolation) — a textbook command injection kept alive only by its own test. The production installer (api/plugins/install.ts + github-source-cache) supersedes it with argv-array git, ref validation, and path containment. Audit finding: sec-injection-supplychain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET /api/agents/avatar joined the query id straight into a filesystem path — `?id=../<dir>` resolved outside the agents root (bounded by the fixed avatar.jpg suffix, but still the one route skipping the containment discipline every other id-consuming route follows). Safe-slug regex → 400; traversal proven by a failing test first. Audit finding: sec-traversal. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GET/PUT /api/plugin-settings/{pluginId} used the last path segment
unvalidated as a filename stem. Defense-in-depth: Node URL normalization
already blocks traversal, but this was the one plugin-id route without
the canonical PLUGIN_ID_RE guard. Reuses the exported regex from
@bakin/core/plugins/manifest. Audit finding: sec-traversal (P2 pull-in).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Billed-image dedup rows stored the full ExecToolResult — including up to 500 chars of prompt text and the provider's commentary — violating the ledger's "coordination facts only, never content" invariant. prompt and providerText are now stripped at the persistence boundary; asset identity + promptHash remain, so replays still return the right asset for $0 and the caller's first-run result is unchanged. Existing rows written before this commit retain their content (single-user box; new writes hold the invariant). Audit finding: sec-secrets-ssrf (P2 pull-in). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
settings.json carried search.settings.auth.password and GET /api/settings returned it unredacted — contradicting the secret store's own invariant (secrets never ride a settings broadcast/export). The password now lives in the 0600 secrets.json under providers.antfly (env ANTFLY_PASSWORD overrides), settings keep only the username, and createAppServices injects the resolved credential at adapter init. A one-time boot migration relocates a legacy password and strips it from settings.json. The antfly adapter is untouched (boundary holds). Partial settings-module mocks across tests/ gained the resetSettingsCache export app-services now imports. Audit finding: sec-secrets-ssrf (P2 pull-in). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…p dist trust trustExistingDist let github installs/upgrades and the boot sweep activate a shipped dist/ that was never rebuilt from the import-validated source — the validator ran against source while the executed artifact came from the tarball. The historical justification (consumer machines couldn't resolve SDK sources for a server rebuild) is gone: Whiskit builds from the compiled binary via system bun and resolves the SDK through the plugin's own node_modules. github/local installs now always build from source; the mtime freshness check remains as a pure cache; provenance-verified Whiskit artifact installs keep skipping the build step before this layer. Audit finding: sec-injection-supplychain (P2 pull-in, verified post-audit). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…port The buildAllUserPlugins lockfile read lost its last consumer with the dist-trust removal; the health plugin's HealthRepairHandler type import was already unused on main and blocked the lint gate. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
markhayden
added a commit
that referenced
this pull request
Jun 12, 2026
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
First execution PR from the audit (
.claude/specs/audit-2026-06/REPORT.md, plan in #496). Six independent findings, one revertable commit each — any single commit can begit reverted cleanly.src/core/plugin-installer.tscarried a textbook shell injection (execSyncstring interpolation of caller input), kept re-adoptable only by its own test. Both deleted.GET /api/agents/avatar?id=../…resolved outside the agents root. Safe-slug validation, proven by a failing test first.PLUGIN_ID_REcheck (reused from@bakin/core/plugins/manifest).settings.jsonandGET /api/settingsunredacted. Now in 0600secrets.jsonunderproviders.antfly(envANTFLY_PASSWORDoverrides), injected at adapter init; one-time boot migration relocates a legacy value. Adapter package untouched. Search knowledge doc updated.trustExistingDistdeleted; the import validator and the executed artifact now always come from the same source tree. Provenance-verified Whiskit artifact installs are unaffected. (Verified pre-removal that the original constraint — no SDK resolution on consumer machines — no longer holds.)Verification
bun run test(4,972 pass / 0 fail),bun run typecheck,bun run lint— green per commitbun run build— all three platform binaries compileBAKIN_HOME: server serves 200, the live antfly migration ran end-to-end (password stripped from settings.json, secrets.json created 0600,/api/settingsresponse clean)Refs #496.
🤖 Generated with Claude Code