Fix bare-verb severity tier + line locator for bunx / npm exec / yarn dlx

dist/detectors/claude-settings.js

@@ -1,3 +1,3 @@
 import { configPath, isRecord, lineOfJsonStringValue, readJsonObjectWithSource } from '../discovery.js';
 export const CLAUDE_SETTINGS_FILE = '.claude/settings.json';
 export const CLAUDE_TARGET_PATHS = [CLAUDE_SETTINGS_FILE];
@@ -205,7 +205,17 @@
 }
 function severityForAllow(permission) {
     const normalized = permission.toLowerCase();
-    if (normalized.includes('bash(') || normalized.includes('write(') || normalized.includes('edit(')) {
+    // Match bare verbs (`Bash`, `Write`, `Edit`) and parenthesized
+    // scoped grants (`Bash(npm *)`, `Write(./foo)`, `Edit(...)`)
+    // uniformly. The previous `includes('bash(')` check required the
+    // opening paren, so bare `"Bash"` — which `isBroadAllow` now
+    // correctly flags as broad — silently fell to `medium` severity
+    // despite granting unlimited shell execution.
+    //
+    // `read` stays medium even when bare/wildcard — read access is
+    // less destructive than execute/modify. Bare `Read` still surfaces
+    // as a finding via `isBroadAllow`, just at medium severity.
+    if (/\b(bash|write|edit)\b/.test(normalized)) {
         return 'high';
     }
     return 'medium';

dist/detectors/mcp.js

@@ -1,3 +1,3 @@
 import { readdir } from 'node:fs/promises';
 import { configPath, isRecord, lineOfJsonKey, lineOfJsonStringValue, readJsonObjectWithSource } from '../discovery.js';
 import { isPipeToShellCommand, isUnpinnedCommand, serverCommand, remoteEndpoint, isRemoteEndpoint, isUnencryptedEndpoint } from '../mcp-risk.js';
@@ -290,9 +290,24 @@
     if (/\b(curl|iwr|invoke-webrequest)\b/.test(normalized)) {
         return firstLineForValues(server, [server.command, ...(server.args ?? []), server.url, server.serverUrl]);
     }
-    if (['npx', 'uvx', 'pipx'].includes((server.command ?? '').toLowerCase())) {
+    const cmd = (server.command ?? '').toLowerCase();
+    // Direct runners: the package name lives in args[0..]. `bunx` was
+    // added to `isUnpinnedCommand` in fb56768 but not to the line
+    // locator — bunx findings fell back to the server declaration's
+    // line instead of pointing at the package.
+    if (['npx', 'uvx', 'pipx', 'bunx'].includes(cmd)) {
         return firstLineForValues(server, server.args ?? [], (arg) => looksLikePackageName(arg) && !hasExactVersion(arg));
     }
+    // Wrapper runners: args[0] is the executor subcommand (`exec`,
+    // `dlx`, `x`). Skip it before locating — `exec` and `dlx` both
+    // pass `looksLikePackageName`, so a naive scan would mis-locate
+    // to the subcommand line instead of the package.
+    if (['npm', 'yarn', 'pnpm'].includes(cmd)) {
+        const args = server.args ?? [];
+        if (args.length > 1 && isWrapperSubcommand(cmd, args[0])) {
+            return firstLineForValues(server, args.slice(1), (arg) => looksLikePackageName(arg) && !hasExactVersion(arg));
+        }
+    }
     if (/https:\/\/github\.com\/[^ ]+/.test(normalized)) {
         return firstLineForValues(server, [server.url, server.serverUrl, ...(server.args ?? [])], (value) => value.toLowerCase().includes('https://github.com/'));
     }
@@ -301,6 +316,16 @@
 function lineForRemoteEndpoint(server) {
     return firstLineForValues(server, [server.url, server.serverUrl], isRemoteEndpoint);
 }
+function isWrapperSubcommand(cmd, arg) {
+    const sub = arg.toLowerCase();
+    if (cmd === 'npm')
+        return sub === 'exec' || sub === 'x';
+    if (cmd === 'yarn')
+        return sub === 'dlx';
+    if (cmd === 'pnpm')
+        return sub === 'dlx' || sub === 'exec' || sub === 'x';
+    return false;
+}
 function firstLineForValues(server, values, predicate = () => true) {
     const source = getSourceText(server);
     for (const value of values) {

src/detectors/claude-settings.ts

@@ -1,3 +1,3 @@
 import { configPath, isRecord, lineOfJsonStringValue, readJsonObjectWithSource } from '../discovery.js';
 import type { Finding, Severity } from '../types.js';
 
@@ -240,7 +240,17 @@
 
 function severityForAllow(permission: string): Severity {
   const normalized = permission.toLowerCase();
-  if (normalized.includes('bash(') || normalized.includes('write(') || normalized.includes('edit(')) {
+  // Match bare verbs (`Bash`, `Write`, `Edit`) and parenthesized
+  // scoped grants (`Bash(npm *)`, `Write(./foo)`, `Edit(...)`)
+  // uniformly. The previous `includes('bash(')` check required the
+  // opening paren, so bare `"Bash"` — which `isBroadAllow` now
+  // correctly flags as broad — silently fell to `medium` severity
+  // despite granting unlimited shell execution.
+  //
+  // `read` stays medium even when bare/wildcard — read access is
+  // less destructive than execute/modify. Bare `Read` still surfaces
+  // as a finding via `isBroadAllow`, just at medium severity.
+  if (/\b(bash|write|edit)\b/.test(normalized)) {
     return 'high';
   }
 

src/detectors/mcp.ts

@@ -1,3 +1,3 @@
 import { readdir } from 'node:fs/promises';
 import { configPath, isRecord, lineOfJsonKey, lineOfJsonStringValue, readJsonObjectWithSource } from '../discovery.js';
 import {
@@ -353,9 +353,24 @@
     return firstLineForValues(server, [server.command, ...(server.args ?? []), server.url, server.serverUrl]);
   }
 
-  if (['npx', 'uvx', 'pipx'].includes((server.command ?? '').toLowerCase())) {
+  const cmd = (server.command ?? '').toLowerCase();
+  // Direct runners: the package name lives in args[0..]. `bunx` was
+  // added to `isUnpinnedCommand` in fb56768 but not to the line
+  // locator — bunx findings fell back to the server declaration's
+  // line instead of pointing at the package.
+  if (['npx', 'uvx', 'pipx', 'bunx'].includes(cmd)) {
     return firstLineForValues(server, server.args ?? [], (arg) => looksLikePackageName(arg) && !hasExactVersion(arg));
   }
+  // Wrapper runners: args[0] is the executor subcommand (`exec`,
+  // `dlx`, `x`). Skip it before locating — `exec` and `dlx` both
+  // pass `looksLikePackageName`, so a naive scan would mis-locate
+  // to the subcommand line instead of the package.
+  if (['npm', 'yarn', 'pnpm'].includes(cmd)) {
+    const args = server.args ?? [];
+    if (args.length > 1 && isWrapperSubcommand(cmd, args[0])) {
+      return firstLineForValues(server, args.slice(1), (arg) => looksLikePackageName(arg) && !hasExactVersion(arg));
+    }
+  }
 
   if (/https:\/\/github\.com\/[^ ]+/.test(normalized)) {
     return firstLineForValues(server, [server.url, server.serverUrl, ...(server.args ?? [])], (value) =>
@@ -370,6 +385,14 @@
   return firstLineForValues(server, [server.url, server.serverUrl], isRemoteEndpoint);
 }
 
+function isWrapperSubcommand(cmd: string, arg: string): boolean {
+  const sub = arg.toLowerCase();
+  if (cmd === 'npm') return sub === 'exec' || sub === 'x';
+  if (cmd === 'yarn') return sub === 'dlx';
+  if (cmd === 'pnpm') return sub === 'dlx' || sub === 'exec' || sub === 'x';
+  return false;
+}
+
 
 function firstLineForValues(
   server: McpServerModel,

test/heuristics-and-snapshot.test.mjs

@@ -68,6 +68,28 @@ test('isBroadAllow: bare Bash/Read/Write/Edit ARE broad (regression for security
   assert.equal(isBroadAllow('Edit'), true);
 });
 
+test('Claude detector: bare Bash/Write/Edit get high severity (not medium)', async () => {
+  // Pre-fix gap: `severityForAllow` required the opening paren
+  // (`bash(`, `write(`, `edit(`) to assign `high`, so bare `"Bash"`
+  // — which grants unlimited shell execution — silently dropped to
+  // `medium`. Bare `Read` deliberately stays medium because read
+  // access is less destructive than execute/modify.
+  const dir = await makeClaudeFixture(
+    { permissions: { allow: [], deny: [] }, hooks: {} },
+    { permissions: { allow: ['Bash', 'Write', 'Edit', 'Read'], deny: [] }, hooks: {} }
+  );
+  try {
+    const findings = await detectClaudeSettingsDrift(dir.oldRoot, dir.newRoot);
+    const bySubject = Object.fromEntries(findings.map((f) => [f.subject, f]));
+    assert.equal(bySubject['Bash'].severity, 'high', 'bare Bash should be high');
+    assert.equal(bySubject['Write'].severity, 'high', 'bare Write should be high');
+    assert.equal(bySubject['Edit'].severity, 'high', 'bare Edit should be high');
+    assert.equal(bySubject['Read'].severity, 'medium', 'bare Read stays medium by design');
+  } finally {
+    await rm(dir.root, { recursive: true, force: true });
+  }
+});
+
 test('isBroadAllow: narrowly-scoped Bash/Read/Write/Edit stay narrow', () => {
   // The bare-verb fix must not over-fire on legitimate narrow scopes.
   assert.equal(isBroadAllow('Bash(npm test)'), false);

test/mcp-drift.test.mjs

@@ -46,6 +46,73 @@ test('mcp_sample_remote_endpoint: http:// fires high severity, https:// stays me
   }
 });
 
+test('lineForUnpinnedCommand pinpoints package line for bunx + npm exec / yarn dlx / pnpm dlx', async () => {
+  // Pre-fix gap: `lineForUnpinnedCommand` only mapped npx/uvx/pipx,
+  // so bunx findings (and the wrapper-runners npm/yarn/pnpm with
+  // exec/dlx subcommands) fell back to the server-declaration line
+  // instead of pointing at the package the reviewer needs to see.
+  //
+  // For wrappers we also have to skip args[0] (the subcommand) —
+  // `exec` and `dlx` both pass `looksLikePackageName`, so a naive
+  // scan would mis-locate to the subcommand line.
+  const { mkdtempSync, writeFileSync, mkdirSync, rmSync } = await import('node:fs');
+  const { tmpdir } = await import('node:os');
+
+  const root = mkdtempSync(join(tmpdir(), 'scopetrail-line-'));
+  try {
+    const oldDir = join(root, 'old');
+    const newDir = join(root, 'new');
+    mkdirSync(oldDir, { recursive: true });
+    mkdirSync(newDir, { recursive: true });
+    writeFileSync(join(oldDir, '.mcp.json'), JSON.stringify({ mcpServers: {} }));
+    // Hand-written JSON so we know exact line numbers — package
+    // string for each server is on its own line.
+    const content = [
+      '{',
+      '  "mcpServers": {',
+      '    "bun-server": {',
+      '      "command": "bunx",',
+      '      "args": [',
+      '        "@vendor/bun-pkg"',
+      '      ]',
+      '    },',
+      '    "npm-server": {',
+      '      "command": "npm",',
+      '      "args": [',
+      '        "exec",',
+      '        "@vendor/npm-pkg"',
+      '      ]',
+      '    },',
+      '    "yarn-server": {',
+      '      "command": "yarn",',
+      '      "args": [',
+      '        "dlx",',
+      '        "@vendor/yarn-pkg"',
+      '      ]',
+      '    }',
+      '  }',
+      '}',
+      ''
+    ].join('\n');
+    writeFileSync(join(newDir, '.mcp.json'), content);
+
+    const findings = await detectMcpDrift(oldDir, newDir);
+    const unpinned = findings.filter((f) => f.kind === 'scope_trail.unpinned_mcp_command');
+    const bySubject = Object.fromEntries(unpinned.map((f) => [f.subject, f]));
+
+    assert.ok(bySubject['bun-server'], 'expected unpinned finding for bunx');
+    assert.equal(bySubject['bun-server'].line, 6, 'bunx package is on line 6');
+
+    assert.ok(bySubject['npm-server'], 'expected unpinned finding for npm exec');
+    assert.equal(bySubject['npm-server'].line, 13, 'npm exec package is on line 13 (skips exec subcommand on line 12)');
+
+    assert.ok(bySubject['yarn-server'], 'expected unpinned finding for yarn dlx');
+    assert.equal(bySubject['yarn-server'].line, 20, 'yarn dlx package is on line 20 (skips dlx subcommand on line 19)');
+  } finally {
+    rmSync(root, { recursive: true, force: true });
+  }
+});
+
 test('isUnpinnedCommand flags bunx packages without exact versions', async () => {
   // bunx is the Bun equivalent of npx and ships as a standalone
   // binary, so MCP configs use `"command": "bunx"` directly. Prior