feat: add suppressor to ignore findings in specific scenarios

.changeset/spicy-donkeys-win.md

@@ -0,0 +1,5 @@
+---
+"layne": minor
+---
+
+Adds a new suppressor feature to ignore findings with a "// SECURITY: XYZ" comment

docs/configuration.md

@@ -228,6 +228,63 @@ Arguments are passed directly via `execFile` — **not** through a shell — so
 
 ---
 
+## Finding Suppression
+
+### Why `// nosemgrep` is disabled
+
+Layne passes `--disable-nosemgrep` to Semgrep on every scan (set in `$global.semgrep.extraArgs`). This prevents contributors from silencing findings inline without review — adding `// nosemgrep` in a PR would suppress the finding in that exact PR, bypassing the security review gate entirely.
+
+### The replacement: `// SECURITY: <reason>`
+
+Place a comment with a non-empty justification on the **same line** as the flagged code, or on the **line immediately above** it:
+
+```js
+// SECURITY: This eval call only runs trusted internal templates, never user input.
+eval(internalTemplate);
+
+const query = `SELECT * FROM users WHERE id = ${id}`; // SECURITY: id is always cast to integer by the ORM layer.
+```
+
+```yaml
+# SECURITY: This token is intentionally committed — it is a public read-only CI token with no write access.
+GITHUB_TOKEN: ghp_...
+```
+
+### The tamper-proof guarantee
+
+The suppressor reads each file at the **merge-base SHA** of the PR (the three-dot diff base) via `git show`. A `// SECURITY:` comment added in the **current PR** is invisible to the suppressor — it is not in the base. The comment must have been reviewed, approved, and merged in a **previous PR** before it takes effect.
+
+This means a contributor cannot self-approve a finding by adding the comment in their own PR.
+
+### Syntax rules
+
+- Comment style: `//` (JS/TS/Go/Java/C…) or `#` (YAML/shell/Python/Ruby…)
+- The colon must be followed by at least one non-whitespace character: `// SECURITY: reason` ✓, `// SECURITY:` ✗
+- Placement: same line as the finding **or** the line immediately above
+
+### Workflow
+
+1. Review the finding and decide it is a genuine false positive.
+2. Add a `// SECURITY: <justification>` comment explaining why.
+3. Submit a **separate PR** for the suppression comment, have it reviewed and merged.
+4. From that point forward, any PR that triggers the same finding on that line will have it suppressed automatically.
+
+### Keeping `--disable-nosemgrep` in per-repo `extraArgs`
+
+`--disable-nosemgrep` is set in `$global.semgrep.extraArgs` and must be carried into any per-repo `extraArgs` override. If you set per-repo `extraArgs` without including `--disable-nosemgrep`, the flag will be absent for that repo — `extraArgs` fully replaces the default, it does not extend it (see [Replacement, not extension](#per-repo-configuration)).
+
+```json
+{
+  "owner/repo": {
+    "semgrep": {
+      "extraArgs": ["--config", "p/owasp-top-ten", "--severity", "ERROR", "--disable-nosemgrep"]
+    }
+  }
+}
+```
+
+---
+
 ## Labels
 
 Layne can automatically add and remove GitHub labels on a PR based on the scan result. This gives at-a-glance triage context directly in the PR list view without opening each PR.

docs/security-architecture.md

@@ -122,6 +122,16 @@ The worker container has no outbound network restrictions by default. Semgrep's
 
 ---
 
+## Finding Suppression
+
+Semgrep's built-in `// nosemgrep` mechanism is disabled via `--disable-nosemgrep` on every scan. This prevents contributors from self-approving a finding inline — adding `// nosemgrep` in a PR would suppress the finding in that exact PR, bypassing the security review gate.
+
+The replacement is the `// SECURITY: <reason>` comment. It is tamper-proof: the suppressor (`src/suppressor.js`) reads each file at the **merge-base SHA** of the PR via `git show` and checks whether the comment existed there. A `// SECURITY:` comment introduced in the current PR is not present at the merge base, so it has no effect. The comment must have been merged in a previous PR — reviewed and approved — before it suppresses anything.
+
+The suppressor runs in the worker after `dispatch()` returns findings and before `buildAnnotations()` is called. A suppressed finding is removed from the list entirely and never appears in the GitHub Check Run.
+
+---
+
 ## Reporting a Vulnerability in Layne
 
 See [SECURITY.md](../SECURITY.md).

src/__tests__/suppressor.test.js

@@ -0,0 +1,121 @@
+import { describe, it, expect, vi, beforeEach } from 'vitest';
+
+const mockExecFile = vi.fn();
+vi.mock('child_process', () => ({ execFile: mockExecFile }));
+
+const { suppressFindings } = await import('../suppressor.js');
+
+// Helper: make execFile resolve with given stdout
+function resolveWith(stdout) {
+  mockExecFile.mockImplementationOnce((_cmd, _args, cb) => cb(null, stdout, ''));
+}
+
+// Helper: make execFile reject (simulates new file / blob unavailable)
+function rejectWith(err = new Error('not found')) {
+  mockExecFile.mockImplementationOnce((_cmd, _args, cb) => cb(err, '', ''));
+}
+
+const BASE = { workspacePath: '/workspace', baseSha: 'base-sha' };
+
+const finding = (overrides = {}) => ({
+  file: 'src/app.js',
+  line: 5,
+  severity: 'high',
+  message: 'issue',
+  ruleId: 'r/1',
+  tool: 'semgrep',
+  ...overrides,
+});
+
+describe('suppressFindings()', () => {
+  beforeEach(() => vi.clearAllMocks());
+
+  it('returns [] and never calls execFile for an empty findings array', async () => {
+    const result = await suppressFindings([], BASE);
+    expect(result).toEqual([]);
+    expect(mockExecFile).not.toHaveBeenCalled();
+  });
+
+  it('keeps a finding when git show fails (new file)', async () => {
+    rejectWith(new Error('fatal: path not in tree'));
+    const f = finding();
+    const result = await suppressFindings([f], BASE);
+    expect(result).toEqual([f]);
+  });
+
+  it('keeps a finding when there is no SECURITY: comment at base', async () => {
+    resolveWith('line1\nline2\nline3\nline4\nline5\nline6\n');
+    const f = finding({ line: 5 });
+    const result = await suppressFindings([f], BASE);
+    expect(result).toEqual([f]);
+  });
+
+  it('suppresses a finding when // SECURITY: reason is on the same line', async () => {
+    resolveWith('line1\nline2\nline3\nline4\nconst x = 1; // SECURITY: reviewed by alice\nline6\n');
+    const result = await suppressFindings([finding({ line: 5 })], BASE);
+    expect(result).toEqual([]);
+  });
+
+  it('suppresses a finding when // SECURITY: reason is on the line immediately above', async () => {
+    resolveWith('line1\nline2\nline3\n// SECURITY: approved in issue #42\nconst x = 1;\nline6\n');
+    const result = await suppressFindings([finding({ line: 5 })], BASE);
+    expect(result).toEqual([]);
+  });
+
+  it('suppresses a finding with # SECURITY: (YAML/shell comment style)', async () => {
+    resolveWith('line1\nline2\nline3\nline4\n# SECURITY: checked for injection\nline6\n');
+    const result = await suppressFindings([finding({ line: 5 })], BASE);
+    expect(result).toEqual([]);
+  });
+
+  it('does NOT suppress when // SECURITY: has no text after the colon', async () => {
+    resolveWith('line1\nline2\nline3\nline4\n// SECURITY:\nline6\n');
+    const f = finding({ line: 5 });
+    const result = await suppressFindings([f], BASE);
+    expect(result).toEqual([f]);
+  });
+
+  it('calls execFile only once for two findings in the same file (cache hit)', async () => {
+    resolveWith('line1\nline2\nline3\nline4\nline5\n');
+    const f1 = finding({ line: 2 });
+    const f2 = finding({ line: 4 });
+    await suppressFindings([f1, f2], BASE);
+    expect(mockExecFile).toHaveBeenCalledTimes(1);
+  });
+
+  it('does not crash and keeps the finding when finding.line === 1', async () => {
+    resolveWith('const x = 1;\nline2\n');
+    const f = finding({ line: 1 });
+    const result = await suppressFindings([f], BASE);
+    expect(result).toEqual([f]);
+  });
+
+  it('returns the correct filtered array for a mixed batch', async () => {
+    // file A: has a SECURITY comment above line 3
+    const fileAContent = 'line1\n// SECURITY: reviewed by bob\nconst y = 2;\nline4\n';
+    // file B: no SECURITY comment
+    const fileBContent = 'line1\nline2\nline3\n';
+
+    mockExecFile
+      .mockImplementationOnce((_cmd, _args, cb) => cb(null, fileAContent, ''))
+      .mockImplementationOnce((_cmd, _args, cb) => cb(null, fileBContent, ''));
+
+    const fA = finding({ file: 'src/a.js', line: 3 }); // should be suppressed
+    const fB = finding({ file: 'src/b.js', line: 2 }); // should be kept
+
+    const result = await suppressFindings([fA, fB], BASE);
+    expect(result).toEqual([fB]);
+  });
+
+  it('handles git show failure for file A independently from success for file B', async () => {
+    rejectWith(new Error('not found'));
+    resolveWith('line1\nline2\nline3\n');
+
+    const fA = finding({ file: 'src/a.js', line: 2 });
+    const fB = finding({ file: 'src/b.js', line: 2 });
+
+    const result = await suppressFindings([fA, fB], BASE);
+    // fA kept (git show failed), fB kept (no SECURITY comment)
+    expect(result).toEqual([fA, fB]);
+  });
+});

src/__tests__/worker.test.js

@@ -80,12 +80,17 @@ vi.mock('../notifiers/index.js', () => ({
   notify: vi.fn().mockResolvedValue(undefined),
 }));
 
+vi.mock('../suppressor.js', () => ({
+  suppressFindings: vi.fn(async (findings) => findings),
+}));
+
 const { Worker: MockWorker }              = await import('bullmq');
 const { getInstallationToken }            = await import('../auth.js');
 const { startCheckRun, completeCheckRun, ensureLabelsExist, setLabels, getMergeBaseSha } = await import('../github.js');
 const { scanTotal, scanDuration, scanTimeoutsTotal, scanRetriesTotal, findingTotal, findingsPerScan } = await import('../metrics.js');
 const { createWorkspace, setupRepo, getChangedFiles, checkoutFiles, cleanupWorkspace } = await import('../fetcher.js');
 const { dispatch }                        = await import('../dispatcher.js');
+const { suppressFindings }               = await import('../suppressor.js');
 const { loadScanConfig }                  = await import('../config.js');
 const { notify }                          = await import('../notifiers/index.js');
 const { redis }                           = await import('../queue.js');
@@ -186,6 +191,18 @@ describe('processJob()', () => {
       }));
     });
 
+    it('calls suppressFindings with dispatch output and mergeBaseSha', async () => {
+      const rawFindings = [{ file: 'a.js', line: 1, severity: 'high', message: 'x', ruleId: 'r/1', tool: 'semgrep' }];
+      dispatch.mockResolvedValueOnce(rawFindings);
+
+      await processJob(baseJob);
+
+      expect(suppressFindings).toHaveBeenCalledWith(rawFindings, {
+        workspacePath: '/tmp/layne-test-workspace',
+        baseSha:       'merge-base-sha',
+      });
+    });
+
     it('completes the check run with the reporter output', async () => {
       await processJob(baseJob);
       expect(completeCheckRun).toHaveBeenCalledWith(expect.objectContaining({

src/suppressor.js

@@ -0,0 +1,56 @@
+import { execFile } from 'child_process';
+
+const SECURITY_COMMENT_RE = /(?:\/\/|#)\s*SECURITY:\s+\S/;
+
+function gitShow(workspacePath, baseSha, filePath) {
+  return new Promise((resolve, reject) => {
+    execFile('git', ['-C', workspacePath, 'show', `${baseSha}:${filePath}`],
+      (err, stdout) => { if (err && !stdout) reject(err); else resolve(stdout ?? ''); }
+    );
+  });
+}
+
+/**
+ * Suppresses findings that already have a `// SECURITY: <reason>` comment at
+ * the base SHA — meaning the comment was reviewed and merged in a prior PR.
+ * New `// SECURITY:` comments added in the current PR are invisible to this
+ * function (they're not in base), so self-approval is impossible.
+ */
+export async function suppressFindings(findings, { workspacePath, baseSha }) {
+  if (findings.length === 0) return [];
+
+  const fileCache = new Map();
+
+  async function getLines(filePath) {
+    if (fileCache.has(filePath)) return fileCache.get(filePath);
+    let lines = null;
+    try {
+      const content = await gitShow(workspacePath, baseSha, filePath);
+      lines = content.split('\n');
+    } catch {
+      // New file or blob unavailable — no suppression possible
+    }
+    fileCache.set(filePath, lines);
+    return lines;
+  }
+
+  const kept = [];
+  for (const finding of findings) {
+    const lines = await getLines(finding.file);
+    if (lines === null) {
+      kept.push(finding);
+      continue;
+    }
+
+    const sameLine  = lines[finding.line - 1] ?? '';
+    const lineAbove = lines[finding.line - 2] ?? '';
+
+    if (SECURITY_COMMENT_RE.test(sameLine) || SECURITY_COMMENT_RE.test(lineAbove)) {
+      console.log(`[suppressor] suppressed finding ${finding.file}:${finding.line} [${finding.ruleId}] — SECURITY: comment found at base`);
+    } else {
+      kept.push(finding);
+    }
+  }
+
+  return kept;
+}

src/worker.js

@@ -7,6 +7,7 @@ import { getInstallationToken } from './auth.js';
 import { startCheckRun, completeCheckRun, ensureLabelsExist, setLabels, getMergeBaseSha } from './github.js';
 import { createWorkspace, setupRepo, getChangedFiles, checkoutFiles, cleanupWorkspace } from './fetcher.js';
 import { dispatch } from './dispatcher.js';
+import { suppressFindings } from './suppressor.js';
 import { buildAnnotations } from './reporter.js';
 import { loadScanConfig } from './config.js';
 import { notify } from './notifiers/index.js';
@@ -160,7 +161,8 @@ async function runScan(job) {
 
     const scanConfig = await loadScanConfig({ owner, repo });
 
-    const findings = await dispatch({ workspacePath, baseSha, baseRef, changedFiles, labels, owner, repo });
+    const rawFindings = await dispatch({ workspacePath, baseSha, baseRef, changedFiles, labels, owner, repo });
+    const findings    = await suppressFindings(rawFindings, { workspacePath, baseSha: mergeBaseSha });
     console.log(`[worker] ${findings.length} total finding(s) for ${owner}/${repo} PR #${prNumber} across all tools:`);
     for (const f of findings) {
       console.log(`[worker]   ${f.tool} ${f.severity.toUpperCase()} ${f.file}:${f.line} [${f.ruleId}] ${f.message}`);