Skip to content

fix: self-install deps from script location, remove SessionStart hook#13

Merged
hiskudin merged 5 commits into
mainfrom
fix/self-install-without-session-start
Apr 15, 2026
Merged

fix: self-install deps from script location, remove SessionStart hook#13
hiskudin merged 5 commits into
mainfrom
fix/self-install-without-session-start

Conversation

@hiskudin
Copy link
Copy Markdown
Contributor

@hiskudin hiskudin commented Apr 14, 2026

Summary

  • Moves npm install from SessionStart hook into scan-tool-result.mjs itself, using import.meta.url to resolve the plugin root on disk
  • Removes SessionStart hook from hooks/hooks.json entirely
  • First-run install only happens when node_modules/@stackone/defender doesn't exist; subsequent invocations skip it via a synchronous existsSync check (essentially zero overhead)

Why

The SessionStart hook was unreliable: CLAUDE_PLUGIN_ROOT is not guaranteed to be set at that phase, so npm install would silently fail or install to the wrong directory. Moving install into the scan script itself means it uses a path derived from import.meta.url — always correct regardless of env vars.

Test plan

  • Fresh install: delete node_modules/@stackone/defender, trigger a tool call (e.g. Bash echo hi) — defender installs automatically and scan runs
  • Subsequent calls: existsSync returns true, install is skipped, scan is near-instant
  • Injection content in WebFetch response: blocked with exit 2

🤖 Generated with Claude Code


Summary by cubic

Self-install plugin deps from the scan script using its own path (no env var) and expand scanning to MCP tool responses; warn on install failures. Also remove the SessionStart hook so @stackone/defender installs on first scan only.

  • Bug Fixes

    • Resolve plugin root via import.meta.url; drop CLAUDE_PLUGIN_ROOT.
    • First-run self-install via npm install --prefix "<pluginRoot>" --silent --no-audit --no-fund (120s timeout); on failure, emit stderr warning and skip scan (exit 0); no SessionStart hook.
    • Scan MCP tool outputs using raw.result/raw.output or JSON.stringify(raw) (try/catch), and log serialization errors to stderr to avoid silent skips.
  • Dependencies

    • Switch release-please to the node release type.

Written for commit 95d91a3. Summary will update on new commits.

Move npm install into scan-tool-result.mjs using import.meta.url to
resolve plugin root on disk. This removes the dependency on
CLAUDE_PLUGIN_ROOT being set during SessionStart, which was unreliable
and caused silent install failures. Deps are now installed on first
PostToolUse scan, with a fast existsSync check skipping the install on
all subsequent runs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 14, 2026 16:16
Comment thread scripts/scan-tool-result.mjs Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes the Defender dependency installation self-contained within the PostToolUse scanner script, removing the previously unreliable SessionStart hook-based install step.

Changes:

  • Add first-run dependency self-install logic to scripts/scan-tool-result.mjs by resolving the plugin root from the script location.
  • Remove the SessionStart hook from hooks/hooks.json so installs no longer happen during session initialization.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
scripts/scan-tool-result.mjs Resolves plugin root, self-installs dependencies on first run, and then loads @stackone/defender via createRequire.
hooks/hooks.json Removes the SessionStart hook that previously ran npm install.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/scan-tool-result.mjs
Comment thread scripts/scan-tool-result.mjs Outdated
Comment thread scripts/scan-tool-result.mjs Outdated
Comment thread scripts/scan-tool-result.mjs
MCP tools (Gmail, Notion, etc.) return arbitrary JSON objects rather
than a string or a WebFetch-style {result} wrapper. Previously the
script extracted raw.result ?? raw.output ?? "" which resolved to an
empty string for all MCP responses, silently skipping the scan.

Now falls back to JSON.stringify(raw) so all text fields (body,
snippet, headers, …) are included. Confirmed blocking tier2Score 0.997
on a prompt-injection email via gmail_read_message.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/scan-tool-result.mjs">

<violation number="1" location="scripts/scan-tool-result.mjs:55">
P2: `JSON.stringify(raw)` on arbitrary objects can throw (e.g., circular refs/BigInt), which can crash the hook before scan handling.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread scripts/scan-tool-result.mjs Outdated
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 2 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/scan-tool-result.mjs">

<violation number="1" location="scripts/scan-tool-result.mjs:21">
P1: Avoid trusting `CLAUDE_PLUGIN_ROOT` for plugin root resolution here; prefer the script-derived path so install/load cannot be redirected by environment tampering.</violation>

<violation number="2" location="scripts/scan-tool-result.mjs:33">
P2: On install failure the script exits 0 with no output, silently disabling the entire tool-output scanner. At minimum emit a warning to stderr so users can diagnose why Defender isn't running.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment thread scripts/scan-tool-result.mjs Outdated
Comment thread scripts/scan-tool-result.mjs
- Drop CLAUDE_PLUGIN_ROOT fallback for plugin root resolution; always
  derive from import.meta.url so the path cannot be redirected by env
  var tampering (P1)
- Emit stderr warning on npm install failure instead of silently
  exiting, so users can diagnose why the scanner is disabled (P2)
- Wrap JSON.stringify(raw) in try/catch to handle circular refs or
  BigInt values in MCP tool responses without crashing the hook (P2)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Comment thread scripts/scan-tool-result.mjs Outdated
release-please v17 dropped the generic release type. Switch to node
which natively handles package.json and still supports extra-files
with jsonpath for the plugin JSON files.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 2 files (changes from recent commits).

Requires human review: Changes to dependency installation (moving npm install to the script runtime) and removing the SessionStart hook modify the plugin's lifecycle and warrant human review.

Use process.stderr.write for consistency with the rest of the script.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 issues found across 1 file (changes from recent commits).

Requires human review: Modifies CI/CD release configuration, changes hook architecture, and introduces runtime dependency installation and new serialization logic for security scanning.

@hiskudin hiskudin merged commit 31c6f5a into main Apr 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants