From 4ef45b1a40ddd69be77f50e56448b80bf88419a5 Mon Sep 17 00:00:00 2001 From: oladev2026-tech Date: Sat, 17 Oct 2026 09:58:00 +0000 Subject: [PATCH] feat(findings): secret false-positive suppression, call-graph depth limits, symbol context, worker concurrency Closes #1175 Closes #1166 Closes #1164 Closes #1088 --- apps/api/src/findings/index.ts | 3 + apps/api/src/findings/symbol-context.ts | 156 +++++++++++++++++ .../src/queue/worker-concurrency-config.ts | 140 +++++++++++++++ .../secret-false-positive-suppression.spec.ts | 57 +++++++ .../secret-false-positive-suppression.ts | 108 ++++++++++++ src/analysis/graph/call-graph-depth-limit.ts | 161 ++++++++++++++++++ src/analysis/graph/index.ts | 3 + 7 files changed, 628 insertions(+) create mode 100644 apps/api/src/findings/symbol-context.ts create mode 100644 apps/api/src/queue/worker-concurrency-config.ts create mode 100644 apps/api/src/scanner/secret-false-positive-suppression.spec.ts create mode 100644 apps/api/src/scanner/secret-false-positive-suppression.ts create mode 100644 src/analysis/graph/call-graph-depth-limit.ts diff --git a/apps/api/src/findings/index.ts b/apps/api/src/findings/index.ts index 0da539ea..ca2fb861 100644 --- a/apps/api/src/findings/index.ts +++ b/apps/api/src/findings/index.ts @@ -6,3 +6,6 @@ export * from './findings.repository'; export * from './findings.service'; export * from './findings.controller'; export * from './findings.routes'; + +// Symbol-aware finding context (#1164) +export * from './symbol-context'; diff --git a/apps/api/src/findings/symbol-context.ts b/apps/api/src/findings/symbol-context.ts new file mode 100644 index 00000000..f1751e22 --- /dev/null +++ b/apps/api/src/findings/symbol-context.ts @@ -0,0 +1,156 @@ +/** + * Symbol-aware finding context (#1164). + * + * A finding that only carries a line number is hard to triage: the line may + * have moved, and reviewers need to know which symbol it landed in. This module + * attaches the innermost enclosing symbol to each finding, using the symbol + * spans the parser already produced, so a finding can be reported as + * `Token.transfer` rather than `line 118`. + * + * Spans are validated once and sorted by span size, which makes "innermost" + * well defined: the smallest span that contains the finding's line wins. + */ + +export type SymbolKind = + | "function" + | "method" + | "class" + | "interface" + | "struct" + | "const" + | "unknown"; + +export interface SymbolSpan { + name: string; + kind: SymbolKind; + /** 1-based inclusive start line. */ + startLine: number; + /** 1-based inclusive end line. */ + endLine: number; + /** Enclosing symbol name, when the parser reported one. */ + container?: string; +} + +export interface SymbolContext { + name: string; + kind: SymbolKind; + qualifiedName: string; + startLine: number; + endLine: number; +} + +export interface ContextualFinding { + finding: T; + /** Undefined when the finding is outside every reported symbol. */ + symbol?: SymbolContext; +} + +export class SymbolSpanError extends Error { + constructor(message: string) { + super(message); + this.name = "SymbolSpanError"; + } +} + +export function validateSymbolSpan(span: SymbolSpan): void { + if (!span || typeof span.name !== "string" || span.name.trim() === "") { + throw new SymbolSpanError("symbol name must be a non-empty string"); + } + if (!Number.isInteger(span.startLine) || span.startLine < 1) { + throw new SymbolSpanError(`symbol ${span.name} has an invalid startLine`); + } + if (!Number.isInteger(span.endLine) || span.endLine < span.startLine) { + throw new SymbolSpanError(`symbol ${span.name} ends before it starts`); + } +} + +/** `Container.name` when a container is known, otherwise the bare name. */ +export function qualifiedSymbolName(span: SymbolSpan): string { + const container = span.container?.trim(); + return container ? `${container}.${span.name}` : span.name; +} + +/** + * The innermost symbol containing `line`. + * + * On identical spans the later-declared symbol wins, which matches the common + * case of a nested re-declaration shadowing its parent. + */ +export function findEnclosingSymbol( + symbols: readonly SymbolSpan[], + line: number, +): SymbolSpan | undefined { + if (!Number.isInteger(line) || line < 1) { + throw new RangeError("line must be a positive integer"); + } + + const enclosing = symbols.filter((span) => { + validateSymbolSpan(span); + return span.startLine <= line && line <= span.endLine; + }); + + if (enclosing.length === 0) { + return undefined; + } + + return enclosing.sort( + (left, right) => + left.endLine - left.startLine - (right.endLine - right.startLine) || + right.startLine - left.startLine, + )[0]; +} + +/** Attach symbol context to findings, preserving input order. */ +export function attachSymbolContext( + findings: readonly T[], + symbols: readonly SymbolSpan[], +): Array> { + return findings.map((finding) => { + const span = findEnclosingSymbol(symbols, finding.line); + if (!span) { + return { finding }; + } + + return { + finding, + symbol: { + name: span.name, + kind: span.kind, + qualifiedName: qualifiedSymbolName(span), + startLine: span.startLine, + endLine: span.endLine, + }, + }; + }); +} + +/** Group findings by their enclosing symbol, stable by qualified name. */ +export function groupFindingsBySymbol( + findings: readonly T[], + symbols: readonly SymbolSpan[], +): Array<{ symbol?: string; findings: T[] }> { + const groups = new Map(); + + for (const { finding, symbol } of attachSymbolContext(findings, symbols)) { + const key = symbol?.qualifiedName ?? ""; + groups.set(key, [...(groups.get(key) ?? []), finding]); + } + + return [...groups.entries()] + .map(([symbol, grouped]) => ({ + ...(symbol === "" ? {} : { symbol }), + findings: grouped, + })) + .sort((left, right) => { + if (left.symbol === right.symbol) { + return 0; + } + if (left.symbol === undefined) { + return 1; + } + if (right.symbol === undefined) { + return -1; + } + return left.symbol < right.symbol ? -1 : 1; + }); +} diff --git a/apps/api/src/queue/worker-concurrency-config.ts b/apps/api/src/queue/worker-concurrency-config.ts new file mode 100644 index 00000000..0f1438df --- /dev/null +++ b/apps/api/src/queue/worker-concurrency-config.ts @@ -0,0 +1,140 @@ +/** + * Worker concurrency configuration (#1088). + * + * Concurrency has to be tunable per deployment but must never let a single + * worker exhaust the host. The resolver reads `WORKER_CONCURRENCY` when it is + * set and otherwise derives a default from the CPU count, then clamps the + * result into a safe band. Invalid values fall back to the derived default + * rather than silently taking down the worker. + * + * Everything here is pure: the caller supplies the environment and CPU count. + */ + +export interface WorkerConcurrencyConfig { + /** Jobs one worker processes at the same time. */ + concurrency: number; + /** Lower bound applied to any configured value. */ + minConcurrency: number; + /** Upper bound applied to any configured value. */ + maxConcurrency: number; + /** Concurrent jobs a single repository may occupy. */ + perRepositoryLimit: number; + /** Where the value came from, for logs and startup diagnostics. */ + source: "environment" | "derived"; +} + +export const DEFAULT_WORKER_CONCURRENCY: Omit = { + minConcurrency: 1, + maxConcurrency: 16, + perRepositoryLimit: 2, +}; + +export class WorkerConcurrencyConfigError extends Error { + constructor(message: string) { + super(message); + this.name = "WorkerConcurrencyConfigError"; + } +} + +/** Default concurrency for a host: one job per CPU, clamped to the band. */ +export function derivedConcurrency( + cpuCount: number, + limits: Pick = DEFAULT_WORKER_CONCURRENCY, +): number { + if (!Number.isInteger(cpuCount) || cpuCount <= 0) { + throw new WorkerConcurrencyConfigError("cpuCount must be a positive integer"); + } + return clamp(cpuCount, limits.minConcurrency, limits.maxConcurrency); +} + +export function resolveWorkerConcurrency( + env: Record = {}, + cpuCount = 1, + overrides: Partial> = {}, +): WorkerConcurrencyConfig { + const limits = { ...DEFAULT_WORKER_CONCURRENCY, ...overrides }; + validateLimits(limits); + + const derived = derivedConcurrency(cpuCount, limits); + const raw = env.WORKER_CONCURRENCY?.trim(); + + if (!raw) { + return { ...limits, concurrency: derived, source: "derived" }; + } + + const parsed = Number(raw); + if (!Number.isInteger(parsed) || parsed <= 0) { + return { ...limits, concurrency: derived, source: "derived" }; + } + + return { + ...limits, + concurrency: clamp(parsed, limits.minConcurrency, limits.maxConcurrency), + source: "environment", + }; +} + +/** Split the worker's slots across named queues, largest remainders first. */ +export function distributeConcurrency( + concurrency: number, + queueWeights: Record, +): Record { + if (!Number.isInteger(concurrency) || concurrency <= 0) { + throw new WorkerConcurrencyConfigError("concurrency must be a positive integer"); + } + + const entries = Object.entries(queueWeights).sort(([left], [right]) => + left < right ? -1 : 1, + ); + if (entries.length === 0) { + return {}; + } + + const totalWeight = entries.reduce((sum, [, weight]) => sum + Math.max(0, weight), 0); + if (totalWeight <= 0) { + throw new WorkerConcurrencyConfigError("queueWeights must contain a positive weight"); + } + + const allocation: Record = {}; + let assigned = 0; + const remainders: Array<{ queue: string; remainder: number }> = []; + + for (const [queue, weight] of entries) { + const exact = (concurrency * Math.max(0, weight)) / totalWeight; + const whole = Math.floor(exact); + allocation[queue] = whole; + assigned += whole; + remainders.push({ queue, remainder: exact - whole }); + } + + remainders.sort((left, right) => right.remainder - left.remainder || (left.queue < right.queue ? -1 : 1)); + for (const { queue } of remainders) { + if (assigned >= concurrency) { + break; + } + allocation[queue] += 1; + assigned += 1; + } + + return allocation; +} + +function clamp(value: number, min: number, max: number): number { + return Math.min(max, Math.max(min, value)); +} + +function validateLimits(limits: { + minConcurrency: number; + maxConcurrency: number; + perRepositoryLimit: number; +}): void { + if (!Number.isInteger(limits.minConcurrency) || limits.minConcurrency < 1) { + throw new WorkerConcurrencyConfigError("minConcurrency must be a positive integer"); + } + if (!Number.isInteger(limits.maxConcurrency) || limits.maxConcurrency < limits.minConcurrency) { + throw new WorkerConcurrencyConfigError("maxConcurrency must be >= minConcurrency"); + } + if (!Number.isInteger(limits.perRepositoryLimit) || limits.perRepositoryLimit < 1) { + throw new WorkerConcurrencyConfigError("perRepositoryLimit must be a positive integer"); + } +} diff --git a/apps/api/src/scanner/secret-false-positive-suppression.spec.ts b/apps/api/src/scanner/secret-false-positive-suppression.spec.ts new file mode 100644 index 00000000..9a89f783 --- /dev/null +++ b/apps/api/src/scanner/secret-false-positive-suppression.spec.ts @@ -0,0 +1,57 @@ +import { + filterSuppressedSecrets, + isSuppressedSecret, + type SuppressionInput, +} from "./secret-false-positive-suppression"; + +describe("secret false-positive suppression (#1175)", () => { + const detection = (overrides: Partial = {}): SuppressionInput => ({ + value: "Zx9Qm2Kd7Vp1Lr4Tb8Nw", + filePath: "src/config/secrets.ts", + line: 'const apiKey = "Zx9Qm2Kd7Vp1Lr4Tb8Nw";', + ...overrides, + }); + + it("keeps a credential in first-party source", () => { + expect(isSuppressedSecret(detection())).toEqual({ suppressed: false }); + }); + + it("suppresses test fixtures", () => { + const decision = isSuppressedSecret(detection({ filePath: "src/__tests__/secrets.spec.ts" })); + expect(decision.suppressed).toBe(true); + expect(decision.ruleId).toBe("documentation-path"); + }); + + it("suppresses documentation", () => { + expect(isSuppressedSecret(detection({ filePath: "docs/secrets.md" })).suppressed).toBe(true); + }); + + it("suppresses placeholder values", () => { + const decision = isSuppressedSecret( + detection({ value: "changeme-please", line: 'const token = "changeme-please";' }), + ); + expect(decision.suppressed).toBe(true); + expect(decision.ruleId).toBe("placeholder-value"); + }); + + it("honours the inline allowance marker", () => { + const decision = isSuppressedSecret( + detection({ line: 'const apiKey = "Zx9Qm2Kd7Vp1Lr4Tb8Nw"; // gasguard:allow-secret' }), + ); + expect(decision.suppressed).toBe(true); + expect(decision.ruleId).toBe("inline-allowance"); + }); + + it("splits detections into kept and suppressed without leaking values", () => { + const report = filterSuppressedSecrets([ + { id: "a", ...detection() }, + { id: "b", ...detection({ filePath: "test/fixtures/keys.json" }) }, + ]); + + expect(report.kept.map((item) => item.id)).toEqual(["a"]); + expect(report.suppressed).toEqual([ + { id: "b", ruleId: "documentation-path", reason: "path is documentation or an example fixture" }, + ]); + expect(JSON.stringify(report)).not.toContain("Zx9Qm2Kd7Vp1Lr4Tb8Nw"); + }); +}); diff --git a/apps/api/src/scanner/secret-false-positive-suppression.ts b/apps/api/src/scanner/secret-false-positive-suppression.ts new file mode 100644 index 00000000..d5046958 --- /dev/null +++ b/apps/api/src/scanner/secret-false-positive-suppression.ts @@ -0,0 +1,108 @@ +/** + * Secret detection false-positive suppression (#1175). + * + * A credential detector that fires on documentation, fixtures and placeholder + * values trains reviewers to ignore it. Suppression is therefore explicit and + * auditable: each suppressed match names the rule that suppressed it, and an + * inline `gasguard:allow-secret` marker lets a repository suppress a single + * line without disabling the detector for a whole path. + * + * Matched values are never copied into a suppression result — only the rule id + * and the location travel with the decision. + */ + +export interface SuppressionInput { + /** Detected literal (used for matching only, never returned). */ + value: string; + /** Repository-relative file path. */ + filePath: string; + /** Full line the literal was found on. */ + line: string; +} + +export interface SuppressionRule { + id: string; + reason: string; + test: (input: SuppressionInput) => boolean; +} + +export interface SuppressionDecision { + suppressed: boolean; + ruleId?: string; + reason?: string; +} + +const SUPPRESSIBLE_PATH_PATTERNS = [ + /(^|\/)(tests?|__tests__|fixtures?|examples?|mocks?)\//i, + /\.(md|mdx|rst|txt)$/i, + /(^|\/)docs?\//i, + /\.env\.(example|sample|template)$/i, +]; + +const PLACEHOLDER_VALUE = /(example|placeholder|dummy|sample|changeme|your[-_]?|xxxx|redacted|notreal)/i; +const INLINE_ALLOWANCE = /gasguard:allow-secret/; + +export const DEFAULT_SUPPRESSION_RULES: SuppressionRule[] = [ + { + id: "inline-allowance", + reason: "line carries an explicit gasguard:allow-secret marker", + test: (input) => INLINE_ALLOWANCE.test(input.line), + }, + { + id: "documentation-path", + reason: "path is documentation or an example fixture", + test: (input) => SUPPRESSIBLE_PATH_PATTERNS.some((pattern) => pattern.test(input.filePath)), + }, + { + id: "placeholder-value", + reason: "value is a recognised placeholder", + test: (input) => PLACEHOLDER_VALUE.test(input.value), + }, +]; + +/** Decide whether one detection is a false positive. */ +export function isSuppressedSecret( + input: SuppressionInput, + rules: readonly SuppressionRule[] = DEFAULT_SUPPRESSION_RULES, +): SuppressionDecision { + if (!input || typeof input.filePath !== "string" || typeof input.line !== "string") { + throw new TypeError("suppression input must include filePath and line"); + } + + for (const rule of rules) { + if (rule.test(input)) { + return { suppressed: true, ruleId: rule.id, reason: rule.reason }; + } + } + + return { suppressed: false }; +} + +export interface SuppressionReport { + kept: T[]; + suppressed: Array<{ id: string; ruleId: string; reason: string }>; +} + +/** Split detections into kept and suppressed, preserving input order. */ +export function filterSuppressedSecrets( + detections: readonly T[], + rules: readonly SuppressionRule[] = DEFAULT_SUPPRESSION_RULES, +): SuppressionReport { + const kept: T[] = []; + const suppressed: SuppressionReport["suppressed"] = []; + + for (const detection of detections) { + const decision = isSuppressedSecret(detection, rules); + if (decision.suppressed) { + suppressed.push({ + id: detection.id, + ruleId: decision.ruleId ?? "unknown", + reason: decision.reason ?? "suppressed", + }); + } else { + kept.push(detection); + } + } + + return { kept, suppressed }; +} diff --git a/src/analysis/graph/call-graph-depth-limit.ts b/src/analysis/graph/call-graph-depth-limit.ts new file mode 100644 index 00000000..7f163afc --- /dev/null +++ b/src/analysis/graph/call-graph-depth-limit.ts @@ -0,0 +1,161 @@ +/** + * Call-graph depth limits (#1166). + * + * Recursive call graphs are unbounded in principle and cyclic in practice. The + * traversal below walks breadth-first from a set of roots, stops at a caller + * supplied depth, and records every node it refused to expand, so a report can + * state exactly where it stopped instead of silently truncating. + * + * Deterministic: each frontier is visited in sorted order and nodes appear once + * however many paths reach them. + */ + +export interface CallGraphEdge { + from: string; + to: string; +} + +export interface DepthLimitOptions { + /** Maximum number of edges followed from a root. */ + maxDepth: number; +} + +export interface TraversalResult { + /** Visited node ids in breadth-first order. */ + visited: string[]; + /** Nodes found at the depth limit whose children were not expanded. */ + truncated: Array<{ nodeId: string; depth: number }>; + deepestDepth: number; +} + +export const DEFAULT_CALL_GRAPH_MAX_DEPTH = 12; + +export class CallGraphDepthLimitError extends Error { + constructor(message: string) { + super(message); + this.name = "CallGraphDepthLimitError"; + } +} + +/** Build an adjacency map; duplicate edges collapse. */ +export function buildAdjacency(edges: readonly CallGraphEdge[]): Map { + const adjacency = new Map(); + + for (const edge of edges) { + if (typeof edge?.from !== "string" || typeof edge?.to !== "string") { + throw new CallGraphDepthLimitError("edges must have string from/to fields"); + } + const targets = adjacency.get(edge.from); + if (targets) { + if (!targets.includes(edge.to)) { + targets.push(edge.to); + } + } else { + adjacency.set(edge.from, [edge.to]); + } + } + + for (const targets of adjacency.values()) { + targets.sort(); + } + + return adjacency; +} + +/** + * Traverse the graph from `roots`, stopping `maxDepth` edges deep. + * + * Roots themselves are depth 0. A node whose children were skipped because of + * the limit is reported in `truncated`. + */ +export function traverseWithDepthLimit( + roots: readonly string[], + edges: readonly CallGraphEdge[], + options: Partial = {}, +): TraversalResult { + const maxDepth = options.maxDepth ?? DEFAULT_CALL_GRAPH_MAX_DEPTH; + if (!Number.isInteger(maxDepth) || maxDepth < 0) { + throw new CallGraphDepthLimitError("maxDepth must be a non-negative integer"); + } + + const adjacency = buildAdjacency(edges); + const visited = new Set(); + const truncated: Array<{ nodeId: string; depth: number }> = []; + const order: string[] = []; + let deepestDepth = 0; + + let frontier = [...new Set(roots)].sort(); + for (const node of frontier) { + visited.add(node); + } + + for (let depth = 0; depth <= maxDepth && frontier.length > 0; depth += 1) { + const next: string[] = []; + + for (const nodeId of frontier) { + order.push(nodeId); + deepestDepth = Math.max(deepestDepth, depth); + + const children = adjacency.get(nodeId) ?? []; + const unvisitedChildren = children.filter((child) => !visited.has(child)); + + if (depth === maxDepth) { + if (unvisitedChildren.length > 0) { + truncated.push({ nodeId, depth }); + } + continue; + } + + for (const child of unvisitedChildren) { + visited.add(child); + next.push(child); + } + } + + frontier = [...new Set(next)].sort(); + } + + return { + visited: order, + truncated: truncated.sort((left, right) => + left.depth === right.depth + ? left.nodeId < right.nodeId + ? -1 + : 1 + : left.depth - right.depth, + ), + deepestDepth, + }; +} + +/** True when the graph contains a cycle reachable from `roots`. */ +export function hasReachableCycle( + roots: readonly string[], + edges: readonly CallGraphEdge[], + maxDepth: number = DEFAULT_CALL_GRAPH_MAX_DEPTH, +): boolean { + const adjacency = buildAdjacency(edges); + const visiting = new Set(); + const done = new Set(); + + const walk = (nodeId: string, depth: number): boolean => { + if (visiting.has(nodeId)) { + return true; + } + if (done.has(nodeId) || depth > maxDepth) { + return false; + } + + visiting.add(nodeId); + for (const child of adjacency.get(nodeId) ?? []) { + if (walk(child, depth + 1)) { + return true; + } + } + visiting.delete(nodeId); + done.add(nodeId); + return false; + }; + + return roots.some((root) => walk(root, 0)); +} diff --git a/src/analysis/graph/index.ts b/src/analysis/graph/index.ts index 2eca115a..75104556 100644 --- a/src/analysis/graph/index.ts +++ b/src/analysis/graph/index.ts @@ -1,2 +1,5 @@ export * from './call-graph-builder'; export * from './gas-tree-evaluator'; + +// Call-graph depth limits (#1166) +export * from './call-graph-depth-limit';