feat(analyzer): add OpenCode-native SkillSpector invocation skill and tool - #537
feat(analyzer): add OpenCode-native SkillSpector invocation skill and tool#537Yoseph-Zuskin wants to merge 4 commits into
Conversation
…command - Add .opencode/tools/skillspector_scan.ts: tool wrapping skillspector CLI via execFile with binary chain (SKILLSPECTOR_BIN env -> .venv -> PATH), 120s timeout, stdout/stderr truncation, secret redaction - Add .opencode/commands/skillspector.md: slash command forwarding $ARGUMENTS with LLM opt-in via noLlm flag - Default to --no-llm for instant static scans; LLM opt-in via noLlm=false or env passthrough (SKILLSPECTOR_PROVIDER, SKILLSPECTOR_MODEL) Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com> Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai> Co-Authored-By: OpenCode Nemotron 3 Ultra Free (1M context) <noreply@opencode.ai>
- The host may omit declared arg defaults, so re-apply them in execute(): format falls back to "json" (avoids `--format undefined`, exit 2), noLlm falls back to true (an omitted noLlm would otherwise silently opt into LLM analysis against the static-by-default contract) - Verified headless on opencode 1.18.30: `/skillspector` round-trip green, fixture 0/LOW SAFE Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com> Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
- The host may omit declared arg defaults, so execute() re-applies them: format falls back to "json" (avoids `--format undefined`, exit 2), noLlm falls back to true (an omitted noLlm would otherwise silently opt into LLM analysis against the static-by-default contract) - Extract pure helpers (truncate, redact, resolveBinary, arg building, exit-code mapping) into dependency-free .opencode/tools/skillspector_scan_lib.ts; thin the tool file to wiring - Cover the lib with skillspector_scan_lib.test.ts via stdlib node --test (18/18, zero new deps; Node 22+ type stripping) - Verified: node --test 18/18; headless `/skillspector` round-trips green on opencode 1.18.30, fixture 0/LOW SAFE (twice, including once with the host omitting all defaults) Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com> Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
- Add docs/OPENCODE_EXTENSION.md modeled on docs/PI_EXTENSION.md: requirements, install (copy .opencode/, skillspector on PATH or SKILLSPECTOR_BIN), basic /skillspector scan, tool parameters, env-based LLM opt-in, unit-test invocation, removal - Note the deliberate param differences vs the Pi extension (no provider/model/yaraRulesDir/verbose; LLM via environment) - Link the new doc from the README Documentation section next to the Pi extension entry Signed-off-by: Yoseph Zuskin <zuskinyoseph@gmail.com> Co-Authored-By: OpenCode Muse Spark 1.3 Free (1M context) <noreply@opencode.ai>
rng1995
left a comment
There was a problem hiding this comment.
[SkillSpector Review]
Reviewed current head fc1773a0027f95a92194fe7b9c0c46be2a3bd5d2. The extension is not ready to ship: it allows model-triggered scans of arbitrary host paths and optional LLM egress without an OpenCode permission request, ignores cancellation, mangles supported SCP-style Git targets, documents a nonexistent provider, loses valid exit-2 reports and successful-run warnings, incompletely redacts supported credentials, and its TypeScript tests are not run by required CI. The inline comments give concrete corrections and regressions.
Hosted checks pass, but these required security/correctness/test changes and mergeStateStatus=BEHIND block merging.
|
|
||
| let result: { stdout: string; stderr: string } | ||
| try { | ||
| result = (await runFile(bin, cliArgs, { |
There was a problem hiding this comment.
[P1] Request permission for host access and LLM egress
Custom tools do not inherit OpenCode's built-in read/edit/external-directory permission prompts. This model-callable tool accepts arbitrary absolute input and output paths and can opt into sending skill content to an external LLM, then executes both without context.ask. Require an explicit, narrowly scoped permission request (including external-directory reads/writes and network/LLM egress), or restrict targets to the session tree; add denial-path tests before launching the process.
| timeout: TIMEOUT_MS, | ||
| maxBuffer: 32 * 1024 * 1024, | ||
| cwd: baseDir, | ||
| })) as { stdout: string; stderr: string } |
There was a problem hiding this comment.
[P2] Propagate the OpenCode cancellation signal
context.abort is ignored, so canceling the tool leaves the child scan running until the fixed 120-second timeout; with noLlm=false, that can continue external requests and cost after the user canceled. Pass signal: context.abort to execFile and handle the abort result distinctly in tests.
|
|
||
| export function isUrlOrAbsolute(target: string): boolean { | ||
| return ( | ||
| /^[a-zA-Z][a-zA-Z0-9+.-]*:\/\//.test(target) || path.isAbsolute(target) |
There was a problem hiding this comment.
[P1] Recognize supported SCP-style Git targets
The SkillSpector CLI explicitly accepts git@host:owner/repo.git, but this predicate returns false for that form. execute therefore sends it through path.resolve, turning it into a bogus local path and breaking a documented input class. Recognize the supported SCP-style syntax without broadly accepting arbitrary colon strings, and add a regression.
| .replace(/sk-ant-[A-Za-z0-9_-]+/g, "[REDACTED]") | ||
| .replace(/\bsk-[A-Za-z0-9_-]{6,}\b/g, "[REDACTED]") | ||
| .replace( | ||
| /\b([A-Z][A-Z0-9_]*_(?:API_KEY|TOKEN))(\s*[:=]\s*["']?)[^"'\s,}]+/g, |
There was a problem hiding this comment.
[P1] Redact every supported credential name
The generic pattern only covers names ending in _API_KEY or _TOKEN; it misses SkillSpector's documented NVIDIA_INFERENCE_KEY and AWS's AWS_SECRET_ACCESS_KEY. If either appears in scanner/provider diagnostics, this model-visible tool returns it verbatim. Cover the repository's complete supported credential set (prefer value-based redaction from a tightly selected environment allowlist) and add exact regressions.
| // Findings above the risk threshold: the JSON report is the answer, not a crash. | ||
| return redact(truncate(partialOut, MAX_STDOUT)) | ||
| } | ||
| if (e.code === 2) { |
There was a problem hiding this comment.
[P1] Preserve reports produced before exit 2
Exit 2 is not exclusively a usage error: the scan command writes a valid report first and then exits 2 when execution_successful is false, and recursive scans can likewise emit output before a fatal accounting result. This branch discards partialOut and mislabels the outcome, hiding the evidence needed to diagnose an incomplete security scan. Return bounded/redacted stdout alongside stderr and distinguish usage failures only when actually proven; add the report-plus-exit-2 regression.
| stderr: string, | ||
| ): string { | ||
| if (output && !stdout) return `Report saved to: ${output}` | ||
| return redact(truncate(stdout || stderr, MAX_STDOUT)) |
There was a problem hiding this comment.
[P1] Do not discard successful-run warnings
Whenever stdout is nonempty, this drops all stderr. SkillSpector intentionally emits security-relevant warnings there—for example incomplete discovery and detection of an author-shipped baseline—while still returning a JSON report on stdout. Return independently bounded/redacted stdout and stderr so the caller sees both, and test a successful report accompanied by a warning.
| ``` | ||
|
|
||
| ```bash | ||
| export SKILLSPECTOR_PROVIDER=opencode_cli |
There was a problem hiding this comment.
[P1] Use a provider that SkillSpector actually registers
opencode_cli is not a valid SKILLSPECTOR_PROVIDER; the provider selector rejects it, so the documented LLM-backed example always exits 2 instead of running semantic analysis. Document one of the real configured providers and a compatible model/credential flow, or add and test the provider before advertising it.
| @@ -0,0 +1,158 @@ | |||
| // Unit tests for the OpenCode plugin helpers. Stdlib only: | |||
| // node --test tests/opencode/skillspector_scan_lib.test.ts | |||
There was a problem hiding this comment.
[P1] Run these TypeScript tests in required CI
No required workflow invokes node --test; the repository's make test-ci path runs pytest, and Ruff does not execute or type-check these files. Consequently every helper regression added here can fail while all hosted checks stay green. Pin a supported Node version and wire this test command into a required CI job (including failure propagation).
Add OpenCode-native SkillSpector invocation (
/skillspector+ tool)Problem
There is no OpenCode-native way to invoke SkillSpector: no slash command,
no agent-callable tool, so scans require manual CLI use outside the
session.
Fixes: #535
Approach
(file-based tools/commands auto-discovery handles registration):
.opencode/tools/skillspector_scan.tsand.opencode/commands/skillspector.md.skillspector_scan(target, format=json, noLlm=true, output?)shells out via stdlib
execFile: binary chainSKILLSPECTOR_BINenv,then
<worktree>/.venv, then bareskillspectoron PATH, with aninstall hint on miss; defaults
scan <target> --format <format> --no-llm. Mirrors the precedent ofextensions/skillspector.ts(Pi agent surface: same tool name, redaction, truncation, binary
chain) — same conventions, different host loader.
chars / stderr ~6k;
sk-ant-*,sk-*,*_API_KEY/*_TOKENredacted.crash); 2 returns stderr as a usage error.
/skillspectorforwards$ARGUMENTSas the target with an LLM opt-in line.--no-llmfor instant static scans; LLM opt-in vianoLlm=falseorSKILLSPECTOR_PROVIDER/MODELenv passthrough.Verification
/skillspectorauto-discovered, agent calledskillspector_scan {"noLlm":true,"target":"./fixture-skill"}, JSONflowed back and was summarized as 0/100 LOW, SAFE, 2 files, 100%
coverage.
returns the install hint; timeout kills at 120s with partial output
and redacted secrets; usage error surfaces stderr; exit-1 risk
fixture (score 100/CRITICAL) returns parseable JSON head with
truncation note.
feat/opencode-pluginon origin; self-scan of.opencode/viaopencode_cli: 0/LOW, 0 issues, 3/3 LLM calls.SKILLSPECTOR_BIN→ venvskillspector.exe):/skillspectorauto-discovered, agent called
skillspector_scan, fixture skillreturned 0/100 LOW, SAFE, 100% coverage. Finding: the host may omit
the
formatdefault, so the tool passed--format undefined(exit 2, surfaced cleanly); fixed via
args.format ?? "json"/args.noLlm ?? truefallbacks (the latter keeps LLM strictly opt-in)..opencode/tools/skillspector_scan_lib.ts, covered bytests/opencode/skillspector_scan_lib.test.tsvia stdlibnode --test(18/18, Node 22+ type stripping, zero new deps); headless
/skillspectorregression re-run green after the reshape.
Sample
Risks
came back truncated); consumers needing full JSON should pass
output.execFile, notBun.$, for timeout/kill support; trivialto swap if maintainers prefer
$.Signed-off-by(maintainer: verify on push).