Repository navigation
Install-time trust gate for skills, MCP servers, and package installs (#305) - #661
Merged
currentbits merged 1 commit intoAug 22, 2026
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
currentbits
force-pushed
the
coder/fork-install-time-scanning-for-mcp-serve-937bf7
branch
from
August 22, 2026 14:13
a313797 to
d5f1228
Compare
Owner
Author
currentbits
deleted the
coder/fork-install-time-scanning-for-mcp-serve-937bf7
branch
October 3, 2026 16:07
currentbits
added a commit
that referenced
this pull request
Oct 19, 2026
Co-authored-by: Solenta <solenta@local>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #305.
One policy module,
electron/installScan.js, applied at three install surfaces. Regex-only, local, no network, no ML. Reuses the existingguardrails.jsinjection and secret scanners rather than reimplementing them, and honours the existingCODER_GUARDRAILS=offescape hatch. Every scan fails open — a feed or parse bug must not brick a shell call.Each surface reports
trusted/caution/blocked.blockedrefuses;cautionis shown and the user can proceed.Package installs in agent shells
classifyToolnow routes install commands (npm/pnpm/yarn/bun/npx/bunx, pip/uv/poetry, cargo, gem, go) throughscanPackageInstall. New settingpackageInstallScan:blocklist(default) — deny known-malicious names, allow the restask— prompt on every package install, still deny the blocklistoff— no interceptThe blocklist is a bundled seed of 16 well-known typosquats plus an optional
userData/malware-packages.jsonoverlay (60s TTL).Also adds
run_terminal_commandtoSHELL_TOOLS— that is grok's shell tool name, which was previously not classified at all.MCP servers
Enabling a server scans its URL. Blocked: non-http(s) scheme, credentials in the URL, secret-shaped query string, remote plaintext http. Remote https is
caution(the server sees agent context); localhost is trusted. Only newly enabled servers are checked, so existing config still saves.getSettingsreturns a derivedtrustper server; it is stripped on write and never persisted.Skills
SKILL.mdis scanned on write and on list. Blocked: curl/wget piped to a shell or interpreter, strings that try to disable Solenta guardrails, embedded secrets, prompt-injection patterns, blocklisted package installs. Caution: "run this skill on every request", "rewrite CLAUDE.md/.claude/settings".skills.addtakesforce: trueto override a block; the Skills tab shows a Trusted/Caution/Blocked badge with the findings as its tooltip, and confirms before writing anything non-trusted.Verification
Run on the branch:
npm run typecheck— cleannpx vite build— cleannpm run test:electron— 1884 pass / 0 failnpm run test:renderer— 1476 pass / 0 failNew coverage:
electron/test/install-scan.test.js, plus additions toguardrails.test.js,skills.test.js,settingsModal.test.tsx,skillsTab.test.tsx.Base
Stacked on #663 (reconcile local main with origin/main). Review and merge that first; this PR retargets to
mainautomatically when it lands. Rebased onto the reconciled history, so this diff is now just the security work: +1527/-50 across 28 files.An earlier revision of this PR read +2838/-159 across 56 files because the branch was sitting on 5 unpushed local-main commits. That is fixed, not hidden.
Known ceilings
blockedMCP URL cannot be forced through the UI (skills can be, withforce).🤖 Generated with Claude Code