diff --git a/apps/cli/package.json b/apps/cli/package.json index beeaec0..f3c4f24 100644 --- a/apps/cli/package.json +++ b/apps/cli/package.json @@ -36,6 +36,7 @@ "dependencies": { "@iarna/toml": "^2.2.5", "@sentry/node": "^8.45.0", + "@threatcrush/scan": "workspace:*", "better-sqlite3": "^11.7.0", "blessed": "^0.1.81", "blessed-contrib": "^4.11.0", diff --git a/apps/cli/src/commands/__tests__/scan.test.ts b/apps/cli/src/commands/__tests__/scan.test.ts new file mode 100644 index 0000000..92f8ab5 --- /dev/null +++ b/apps/cli/src/commands/__tests__/scan.test.ts @@ -0,0 +1,21 @@ +import { describe, expect, it } from 'vitest'; +import { parseFailOn } from '../scan.js'; + +/** + * Flag parsing lives here rather than with the engine tests. + * + * `@threatcrush/scan` decides what a finding is and whether a set of them + * clears a threshold; it has no opinion about argv. This is the boundary the + * package extraction drew, and the test placement follows it. + */ +describe('--fail-on', () => { + it('accepts a comma-separated list of severities', () => { + expect(parseFailOn('critical,high')).toEqual(['critical', 'high']); + }); + + it('rejects an unknown severity rather than silently ignoring it', () => { + // Silently accepting `--fail-on hihg` produces a gate that never fires, + // which looks exactly like a passing build. + expect(() => parseFailOn('hihg')).toThrow(/unknown severity/); + }); +}); diff --git a/apps/cli/src/commands/scan.ts b/apps/cli/src/commands/scan.ts index f54241f..6128208 100644 --- a/apps/cli/src/commands/scan.ts +++ b/apps/cli/src/commands/scan.ts @@ -5,11 +5,9 @@ import ora from 'ora'; import { banner, logger } from '../core/logger.js'; import type { RunResult, StructuredFinding } from '../core/run-result.js'; import { summarize } from '../core/run-result.js'; -import { scanDependencies } from '../scan/dependencies.js'; -import { meetsFailThreshold, scanPath } from '../scan/engine.js'; -import { buildSarif } from '../scan/sarif.js'; -import type { ScanFinding, Severity } from '../scan/types.js'; -import { SEVERITY_ORDER } from '../scan/types.js'; +import { meetsFailThreshold, SEVERITY_ORDER } from '@threatcrush/scan'; +import type { ScanFinding, Severity } from '@threatcrush/scan'; +import { buildSarif, scanDependencies, scanPath } from '@threatcrush/scan/node'; export type ScanFormat = 'text' | 'json' | 'sarif'; diff --git a/apps/cli/tsup.config.ts b/apps/cli/tsup.config.ts index f968b02..35654a0 100644 --- a/apps/cli/tsup.config.ts +++ b/apps/cli/tsup.config.ts @@ -17,7 +17,11 @@ export default defineConfig({ dts: false, external: ['better-sqlite3', 'blessed', 'blessed-contrib', 'react', 'react-blessed', 'react-blessed-contrib'], - noExternal: ['chalk', 'ora', '@iarna/toml', 'commander'], + // `@threatcrush/scan` is bundled, not externalised. It resolves to + // TypeScript source rather than a build output — see its package.json — so + // there is nothing for Node to require at runtime, and the published CLI + // must not gain a dependency on a package that is not published. + noExternal: ['chalk', 'ora', '@iarna/toml', 'commander', '@threatcrush/scan'], async onSuccess() { // Ship the systemd unit template alongside the compiled bundle. diff --git a/packages/scan/README.md b/packages/scan/README.md new file mode 100644 index 0000000..56cf685 --- /dev/null +++ b/packages/scan/README.md @@ -0,0 +1,65 @@ +# `@threatcrush/scan` + +The scan rules and engine, shared by the CLI, web, desktop and extension. + +Previously this lived at `apps/cli/src/scan/`, which meant the CLI was the only +surface that could run a scan. Every other app either did without or would have +grown its own copy of the rules. + +## Two entry points + +```ts +import { scanText, CODE_RULES } from '@threatcrush/scan'; // anywhere +import { scanPath, buildSarif } from '@threatcrush/scan/node'; // needs a filesystem +``` + +| entry | contains | runs in | +|---|---|---| +| `.` | rules, `scanText`, language detection, suppressions, severity | browser, worker, Node | +| `./node` | `scanPath` tree walker, dependency scan, SARIF output | Node only | + +The default entry point imports nothing from `node:`. That is enforced by +`src/__tests__/boundaries.test.ts`, not by convention — a browser bundle breaks +at the *consumer's* build if a filesystem import creeps in, which is a failure +that surfaces late and in the wrong repository. + +Verify by hand at any time: + +```sh +npx esbuild src/index.ts --bundle --platform=browser --format=esm --outfile=/dev/null # succeeds +npx esbuild src/node/index.ts --bundle --platform=browser --format=esm --outfile=/dev/null # fails, by design +``` + +## Why `exports` points at TypeScript source + +This is an internal package: `exports` resolves to `src/*.ts` rather than a +build output. + +The alternative — compiling to `dist/` — introduces a build ordering +requirement, and the release workflow runs `pnpm --filter @profullstack/threatcrush build` +alone. A package that had to be built first would publish a broken CLI the +first time someone forgot, and the failure would be a runtime `MODULE_NOT_FOUND` +in the published artefact rather than a red build. + +Consumers therefore transpile it themselves: + +- **CLI** — bundled by tsup via `noExternal`, so the published package stays + self-contained and gains no dependency on an unpublished package. +- **Next.js** (web) — add `transpilePackages: ['@threatcrush/scan']`. +- **Vite** (desktop, extension) — works as-is; Vite transpiles linked workspace + sources by default. + +If this package is ever published standalone, add a build step and switch +`exports` to `dist` with a `publishConfig` override. Nothing else needs to move. + +## Adding a rule + +Rules live in `src/code-rules.ts`, credentials in `src/secret-rules.ts`, +manifests in `src/manifest-rules.ts`. Two invariants are enforced by tests: + +- **Every language the scanner claims must have at least one rule.** `shell` + and `php` were both listed as supported while no rule targeted them, so those + files were read and reported clean whatever they contained. +- **Every rule is tested against the corrected shape as well as the vulnerable + one.** A rule that only fires on bad code is untested against the good code + standing next to it, which is where false positives come from. diff --git a/packages/scan/package.json b/packages/scan/package.json new file mode 100644 index 0000000..04073cb --- /dev/null +++ b/packages/scan/package.json @@ -0,0 +1,29 @@ +{ + "name": "@threatcrush/scan", + "version": "0.7.0", + "description": "ThreatCrush scan rules and engine, shared by the CLI, web, desktop and extension.", + "license": "MIT", + "type": "module", + "exports": { + ".": "./src/index.ts", + "./node": "./src/node/index.ts" + }, + "files": [ + "src" + ], + "scripts": { + "test": "vitest run", + "typecheck": "tsc --noEmit" + }, + "devDependencies": { + "@types/node": "^22.10.1", + "typescript": "^5.6.3", + "vitest": "^3.2.4" + }, + "repository": { + "type": "git", + "url": "git+https://github.com/profullstack/threatcrush.git", + "directory": "packages/scan" + }, + "homepage": "https://threatcrush.com" +} diff --git a/packages/scan/src/__tests__/boundaries.test.ts b/packages/scan/src/__tests__/boundaries.test.ts new file mode 100644 index 0000000..5806928 --- /dev/null +++ b/packages/scan/src/__tests__/boundaries.test.ts @@ -0,0 +1,58 @@ +import { readdirSync, readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; + +/** + * The default entry point must stay free of `node:` imports. + * + * This is the whole reason the package is split the way it is. The web app, + * the extension and the desktop renderer import `@threatcrush/scan` into a + * browser bundle; a single `node:fs` anywhere in that module graph breaks all + * of them, and it breaks at *their* build, not ours — which is the kind of + * failure that gets found late and blamed on the wrong repository. + * + * A comment saying "do not import node: here" does not survive contact with a + * hurried change. This does. + */ + +const SRC = join(__dirname, '..'); + +function sourceFilesOutsideNodeEntry(dir: string, acc: string[] = []): string[] { + for (const entry of readdirSync(dir, { withFileTypes: true })) { + // `node/` is the entry point that is *allowed* to touch the filesystem, + // and `__tests__/` is this file and its neighbours — neither ships to a + // browser. + if (entry.isDirectory()) { + if (entry.name === 'node' || entry.name === '__tests__') continue; + sourceFilesOutsideNodeEntry(join(dir, entry.name), acc); + continue; + } + if (entry.name.endsWith('.ts')) acc.push(join(dir, entry.name)); + } + return acc; +} + +describe('module boundaries', () => { + const files = sourceFilesOutsideNodeEntry(SRC); + + it('finds the source files it is supposed to be checking', () => { + // Guards the guard: a broken walk would make every assertion below pass + // over an empty list. + expect(files.length).toBeGreaterThanOrEqual(5); + }); + + it.each(files.map((f) => [f.slice(SRC.length + 1), f] as const))( + '%s imports nothing from node:', + (_label, file) => { + const offending = readFileSync(file, 'utf-8') + .split('\n') + // Comment lines are skipped, because the first thing this test found + // was the sentence in `text.ts` explaining why `node:fs` must not + // appear there. Matching prose as if it were code is the exact bug the + // scanner's own `proseLines` exists to avoid. + .filter((line) => !/^\s*(?:\/\/|\/?\*)/.test(line)) + .filter((line) => /\bfrom\s+['"]node:/.test(line) || /\brequire\(\s*['"]node:/.test(line)); + expect(offending).toEqual([]); + }, + ); +}); diff --git a/apps/cli/src/scan/__tests__/code-rules.test.ts b/packages/scan/src/__tests__/code-rules.test.ts similarity index 99% rename from apps/cli/src/scan/__tests__/code-rules.test.ts rename to packages/scan/src/__tests__/code-rules.test.ts index 6c5199f..316421e 100644 --- a/apps/cli/src/scan/__tests__/code-rules.test.ts +++ b/packages/scan/src/__tests__/code-rules.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vitest'; import { CODE_RULES, proseLines } from '../code-rules.js'; -import { languageOf, scanText } from '../engine.js'; +import { languageOf, scanText } from '../text.js'; import type { ScanLanguage } from '../types.js'; /** diff --git a/apps/cli/src/scan/__tests__/sarif.test.ts b/packages/scan/src/__tests__/sarif.test.ts similarity index 99% rename from apps/cli/src/scan/__tests__/sarif.test.ts rename to packages/scan/src/__tests__/sarif.test.ts index 5dca05a..d68bf5b 100644 --- a/apps/cli/src/scan/__tests__/sarif.test.ts +++ b/packages/scan/src/__tests__/sarif.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { buildSarif, fingerprintOf, sarifLevel, securitySeverity, toArtifactUri } from '../sarif.js'; +import { buildSarif, fingerprintOf, sarifLevel, securitySeverity, toArtifactUri } from '../node/sarif.js'; import type { ScanFinding } from '../types.js'; const finding = (overrides: Partial = {}): ScanFinding => ({ diff --git a/apps/cli/src/scan/__tests__/engine.test.ts b/packages/scan/src/__tests__/text.test.ts similarity index 95% rename from apps/cli/src/scan/__tests__/engine.test.ts rename to packages/scan/src/__tests__/text.test.ts index 2fc07a9..fecb127 100644 --- a/apps/cli/src/scan/__tests__/engine.test.ts +++ b/packages/scan/src/__tests__/text.test.ts @@ -1,12 +1,11 @@ import { describe, expect, it } from 'vitest'; -import { parseFailOn } from '../../commands/scan.js'; import { collectSuppressions, languageOf, languageOfShebang, meetsFailThreshold, scanText, -} from '../engine.js'; +} from '../text.js'; import { detectTyposquat, editDistance, scanPackageJson, scanRequirementsTxt } from '../manifest-rules.js'; import { isKnownPlaceholder, redactSecret } from '../secret-rules.js'; import type { ScanFinding } from '../types.js'; @@ -130,12 +129,9 @@ describe('--fail-on', () => { expect(meetsFailThreshold([at('critical')], [])).toBe(false); }); - it('rejects an unknown severity rather than silently ignoring it', () => { - // Silently accepting `--fail-on hihg` produces a gate that never fires, - // which looks exactly like a passing build. - expect(parseFailOn('critical,high')).toEqual(['critical', 'high']); - expect(() => parseFailOn('hihg')).toThrow(/unknown severity/); - }); + // Parsing the flag itself is the CLI's job and is tested there — see + // apps/cli/src/commands/__tests__/scan.test.ts. This package has no opinion + // about argv. }); describe('typosquat detection', () => { diff --git a/apps/cli/src/scan/code-rules.ts b/packages/scan/src/code-rules.ts similarity index 100% rename from apps/cli/src/scan/code-rules.ts rename to packages/scan/src/code-rules.ts diff --git a/packages/scan/src/index.ts b/packages/scan/src/index.ts new file mode 100644 index 0000000..a729ac4 --- /dev/null +++ b/packages/scan/src/index.ts @@ -0,0 +1,37 @@ +/** + * `@threatcrush/scan` — the rules and the text engine. + * + * This entry point is deliberately free of `node:` imports. Everything here + * runs in a browser, a service worker or a Node process alike, because the + * whole of it operates on strings that somebody else obtained. That is what + * lets the web app, the extension and the desktop renderer share one copy of + * the rules with the CLI rather than growing their own. + * + * Anything needing a filesystem — walking a tree, reading a manifest off disk, + * writing SARIF — lives behind `@threatcrush/scan/node`. + */ + +export { CODE_RULES, evaluateRule, GENERIC_GUARD, proseLines, untrustedPatternFor } from './code-rules.js'; +export type { CodeRule } from './code-rules.js'; + +export { scanPackageJson, scanRequirementsTxt, detectTyposquat, editDistance } from './manifest-rules.js'; +export type { ManifestFinding, SquatVerdict } from './manifest-rules.js'; + +export { isKnownPlaceholder, redactSecret, SECRET_RULES, SENSITIVE_FILES } from './secret-rules.js'; + +export { + collectSuppressions, + isTestPath, + languageOf, + languageOfShebang, + meetsFailThreshold, + peakSeverity, + SCAN_EXTENSIONS, + scanManifest, + scanText, + SKIP_DIRS, +} from './text.js'; +export type { Suppressions } from './text.js'; + +export { severityRank, SEVERITY_ORDER } from './types.js'; +export type { Confidence, ScanFinding, ScanLanguage, Severity } from './types.js'; diff --git a/apps/cli/src/scan/manifest-rules.ts b/packages/scan/src/manifest-rules.ts similarity index 100% rename from apps/cli/src/scan/manifest-rules.ts rename to packages/scan/src/manifest-rules.ts diff --git a/apps/cli/src/scan/dependencies.ts b/packages/scan/src/node/dependencies.ts similarity index 98% rename from apps/cli/src/scan/dependencies.ts rename to packages/scan/src/node/dependencies.ts index bd6c906..0ad16db 100644 --- a/apps/cli/src/scan/dependencies.ts +++ b/packages/scan/src/node/dependencies.ts @@ -10,7 +10,7 @@ import { existsSync, readFileSync } from 'node:fs'; import { join } from 'node:path'; -import type { ScanFinding, Severity } from './types.js'; +import type { ScanFinding, Severity } from '../types.js'; interface OsvVulnerability { id: string; diff --git a/packages/scan/src/node/index.ts b/packages/scan/src/node/index.ts new file mode 100644 index 0000000..f3e902f --- /dev/null +++ b/packages/scan/src/node/index.ts @@ -0,0 +1,15 @@ +/** + * `@threatcrush/scan/node` — the parts that need a filesystem. + * + * Kept apart from the default entry point so that importing the rules does not + * drag `node:fs` into a browser bundle. Import from here only where a real + * filesystem exists: the CLI, the daemon, a server route. + */ + +export { scanPath } from './walk.js'; +export type { ScanOptions, ScanReport } from './walk.js'; + +export { scanDependencies } from './dependencies.js'; + +export { buildSarif, fingerprintOf, sarifLevel, securitySeverity, toArtifactUri } from './sarif.js'; +export type { SarifOptions } from './sarif.js'; diff --git a/apps/cli/src/scan/sarif.ts b/packages/scan/src/node/sarif.ts similarity index 99% rename from apps/cli/src/scan/sarif.ts rename to packages/scan/src/node/sarif.ts index 3b653ec..27777fc 100644 --- a/apps/cli/src/scan/sarif.ts +++ b/packages/scan/src/node/sarif.ts @@ -26,7 +26,7 @@ import { createHash } from 'node:crypto'; import { isAbsolute, relative, resolve, sep } from 'node:path'; -import type { ScanFinding, Severity } from './types.js'; +import type { ScanFinding, Severity } from '../types.js'; /** * The key our fingerprint is published under. diff --git a/packages/scan/src/node/walk.ts b/packages/scan/src/node/walk.ts new file mode 100644 index 0000000..f9a0c82 --- /dev/null +++ b/packages/scan/src/node/walk.ts @@ -0,0 +1,242 @@ +/** + * The filesystem half of the scan engine: walk a tree, read what is scannable, + * hand the text to the rules. + * + * Split from `../text.ts` because this file imports `node:fs` and that one must + * not. The package's default entry point is consumed by browser bundles, and a + * single filesystem import anywhere in its graph breaks all of them. Everything + * that needs a disk lives behind the `./node` entry point instead. + */ + +import { + closeSync, fstatSync, openSync, readdirSync, readFileSync, readSync, statSync, +} from 'node:fs'; +import { basename, dirname, extname, join, relative, sep } from 'node:path'; +import { SENSITIVE_FILES } from '../secret-rules.js'; +import { + collectSuppressions, + languageOf, + languageOfShebang, + SCAN_EXTENSIONS, + scanManifest, + scanText, + SKIP_DIRS, +} from '../text.js'; +import type { ScanFinding, ScanLanguage } from '../types.js'; +import { severityRank } from '../types.js'; + +export interface ScanOptions { + /** Skip files larger than this. Defaults to 1 MiB. */ + maxFileBytes?: number; + /** Called once per file actually read, for progress reporting. */ + onFile?: (path: string) => void; + /** Restrict to these rule categories. Defaults to all. */ + categories?: readonly ScanFinding['category'][]; +} + +export interface ScanReport { + findings: ScanFinding[]; + filesScanned: number; + /** + * How many inline suppressions were honoured. Reported, never hidden: a + * scan that came back quiet because someone silenced forty rules is a + * different result from a scan that came back quiet. + */ + suppressed: number; + /** + * Directory that finding paths are relative to. Equal to the target for a + * directory scan, its parent for a single-file scan. SARIF URI resolution + * needs this — guessing it from the target is what produces file URIs that + * resolve to nothing. + */ + root: string; + /** + * Files matched by extension but unreadable. Reported rather than swallowed: + * a scan that could not read a file has not cleared it, and "0 findings" + * over an unread tree is the failure this scanner exists to avoid. + */ + unreadable: string[]; +} + +export function scanPath(targetPath: string, options: ScanOptions = {}): ScanReport { + const maxFileBytes = options.maxFileBytes ?? 1024 * 1024; + const allowed = options.categories ? new Set(options.categories) : null; + const findings: ScanFinding[] = []; + const unreadable: string[] = []; + let filesScanned = 0; + let suppressed = 0; + + // A file target is not a degenerate directory target. `readdirSync` on a + // file throws ENOTDIR, which the walker below treats as an unreadable + // directory — so `threatcrush scan app.js` reported a clean scan of a file + // it never opened. Resolve the shape first, and walk only what is walkable. + const rootIsDirectory = (() => { + try { + return statSync(targetPath).isDirectory(); + } catch { + return true; + } + })(); + const walkRoot = rootIsDirectory ? targetPath : dirname(targetPath); + + const scanFile = (fullPath: string, filename: string): void => { + const relativePath = toRelative(walkRoot, fullPath); + const extension = extname(filename).toLowerCase(); + const isManifest = filename === 'package.json' || filename === 'requirements.txt'; + const scannable = SCAN_EXTENSIONS.has(extension) || filename.startsWith('.env'); + + // A file with no extension gets one question asked of it before being + // dismissed: does it start with a shebang? Executables are habitually named + // for what they do rather than what they are written in, and skipping them + // silently is how a repository whose only source file is `debtap` scans + // clean. Files that carry an unrecognised extension are still skipped — + // `.png` is not a script, and sniffing every one of them would mean reading + // the whole tree. + const mayDeclareInterpreter = !scannable && !isManifest && extension === ''; + + if (!scannable && !isManifest && !mayDeclareInterpreter) { + recordSensitiveFile(filename, relativePath, findings, []); + return; + } + + // Size-check and read through one descriptor. + // + // `statSync(path)` followed by `readFileSync(path)` is check-then-use: the + // path can be replaced between the two calls, so the size that was checked + // is not necessarily the size that gets read. Opening once and calling + // `fstatSync` on the descriptor removes the window — the descriptor refers + // to the same inode for both operations, whatever happens to the name. + // + // A scanner walking directories it does not control is exactly where this + // matters, and CWE-362 is a class this tool reports on. Worth getting + // right in its own walker. + let text: string; + let handle: number; + let declared: ScanLanguage | null = null; + try { + handle = openSync(fullPath, 'r'); + } catch { + unreadable.push(relativePath); + return; + } + + try { + if (fstatSync(handle).size > maxFileBytes) return; + + // Sniff the shebang from a short prefix rather than the whole file, so an + // extensionless blob — a checked-in binary, a data file — costs one small + // read instead of a megabyte decoded as UTF-8 and thrown away. + if (mayDeclareInterpreter) { + const prefix = Buffer.alloc(128); + const read = readSync(handle, prefix, 0, prefix.length, 0); + declared = languageOfShebang(prefix.subarray(0, read).toString('utf-8').split('\n', 1)[0] ?? ''); + if (!declared) return; + } + + text = readFileSync(handle, 'utf-8'); + } catch { + unreadable.push(relativePath); + return; + } finally { + try { + closeSync(handle); + } catch { + /* the descriptor is going away regardless */ + } + } + + filesScanned += 1; + options.onFile?.(relativePath); + suppressed += collectSuppressions(text.split('\n')).count; + + const fileFindings = [ + ...scanText(relativePath, text, declared ?? languageOf(filename)), + ...(isManifest ? scanManifest(relativePath, filename, text) : []), + ]; + + findings.push(...fileFindings); + recordSensitiveFile(filename, relativePath, findings, fileFindings); + }; + + const walk = (currentPath: string): void => { + let entries; + try { + entries = readdirSync(currentPath, { withFileTypes: true }); + } catch { + unreadable.push(toRelative(walkRoot, currentPath)); + return; + } + + for (const entry of entries) { + const fullPath = join(currentPath, entry.name); + + if (entry.isDirectory()) { + if (SKIP_DIRS.has(entry.name)) continue; + walk(fullPath); + continue; + } + if (!entry.isFile()) continue; + + scanFile(fullPath, entry.name); + } + }; + + if (rootIsDirectory) { + walk(targetPath); + } else { + scanFile(targetPath, basename(targetPath)); + } + + const filtered = allowed ? findings.filter((f) => allowed.has(f.category)) : findings; + filtered.sort( + (a, b) => + severityRank(b.severity) - severityRank(a.severity) || + a.file.localeCompare(b.file) || + a.line - b.line, + ); + + return { findings: filtered, filesScanned, unreadable, suppressed, root: walkRoot }; +} + +/** + * Report a file whose *name* is the finding — but only when its contents + * produced nothing. + * + * A `.env` full of detected credentials does not also need "this is a .env + * file" stapled to line 1. The filename finding exists for the case the + * content rules cannot cover: an env file whose values are shapes no vendor + * rule matches, which is still an env file that should not be committed. + */ +function recordSensitiveFile( + filename: string, + relativePath: string, + sink: ScanFinding[], + fileFindings: readonly ScanFinding[], +): void { + if (fileFindings.length > 0) return; + + for (const sensitive of SENSITIVE_FILES) { + const matches = filename === sensitive.pattern || filename.endsWith(sensitive.pattern); + if (!matches) continue; + sink.push({ + ruleId: 'sensitive-file-committed', + title: 'Sensitive file', + file: relativePath, + line: 1, + severity: sensitive.severity, + confidence: 'evidence', + message: sensitive.message, + consequence: 'Anything in this file is in every clone, fork and CI cache of the repository.', + cwe: 'CWE-538', + excerpt: '', + sensitive: true, + category: 'file', + }); + return; + } +} + +function toRelative(base: string, target: string): string { + const rel = relative(base, target); + return (rel === '' ? target : rel).split(sep).join('/'); +} diff --git a/apps/cli/src/scan/secret-rules.ts b/packages/scan/src/secret-rules.ts similarity index 100% rename from apps/cli/src/scan/secret-rules.ts rename to packages/scan/src/secret-rules.ts diff --git a/apps/cli/src/scan/engine.ts b/packages/scan/src/text.ts similarity index 55% rename from apps/cli/src/scan/engine.ts rename to packages/scan/src/text.ts index 9a9543c..9ce965b 100644 --- a/apps/cli/src/scan/engine.ts +++ b/packages/scan/src/text.ts @@ -1,29 +1,54 @@ /** - * The scan engine: walk a tree, run every rule set over it, return findings. + * The scan engine, minus the filesystem: run every rule set over text. * - * Kept free of any I/O beyond reading files — no printing, no exit codes, no + * Kept free of I/O entirely — no reading, no printing, no exit codes, no * SARIF. The command layer decides how to present what this returns, which is * what lets the same scan feed a terminal, a SARIF file and a daemon run * without three implementations drifting apart. + * + * The absence of `node:` imports here is load-bearing, not incidental. This + * module is the package's default entry point, and the browser surfaces — + * web, extension, desktop renderer — import it directly. A single + * `import … from 'node:fs'` anywhere in this file's dependency graph breaks + * every one of their bundles, so the tree walker lives in `./node/walk.ts` + * and everything here works on strings that somebody else read. */ -import { - closeSync, fstatSync, openSync, readdirSync, readFileSync, readSync, statSync, -} from 'node:fs'; -import { basename, dirname, extname, join, relative, sep } from 'node:path'; import { CODE_RULES, evaluateRule, proseLines } from './code-rules.js'; import { scanPackageJson, scanRequirementsTxt } from './manifest-rules.js'; import { isKnownPlaceholder, redactSecret, SECRET_RULES, SENSITIVE_FILES } from './secret-rules.js'; import type { ScanFinding, ScanLanguage, Severity } from './types.js'; import { severityRank } from './types.js'; -const SKIP_DIRS = new Set([ +/** + * `extname` and `basename`, reimplemented in three lines each. + * + * Importing them from `node:path` is what would otherwise put this module — + * and therefore the package's whole default entry point — out of reach of a + * browser bundle, for two functions that are pure string arithmetic. + * + * Semantics match the originals on the inputs that reach them: a leading dot + * is not an extension, so `.env` has none, and a name with no dot has none + * either. Only `/` is treated as a separator, which is what the callers pass — + * repository-relative paths and shebang interpreter paths. + */ +function baseNameOf(path: string): string { + return path.slice(path.lastIndexOf('/') + 1); +} + +function extensionOf(path: string): string { + const base = baseNameOf(path); + const dot = base.lastIndexOf('.'); + return dot <= 0 ? '' : base.slice(dot); +} + +export const SKIP_DIRS = new Set([ 'node_modules', '.git', '.next', '.nuxt', 'dist', 'build', 'out', '__pycache__', '.venv', 'venv', 'vendor', '.terraform', 'coverage', '.cache', '.pnpm-store', 'target', '.gradle', '.idea', '.vscode', 'bower_components', '.svelte-kit', ]); -const SCAN_EXTENSIONS = new Set([ +export const SCAN_EXTENSIONS = new Set([ '.ts', '.js', '.tsx', '.jsx', '.mjs', '.cjs', '.mts', '.cts', '.py', '.rb', '.go', '.java', '.kt', '.scala', '.php', '.rs', '.c', '.cc', '.cpp', '.h', '.hpp', '.cs', '.swift', @@ -49,7 +74,7 @@ const LANGUAGE_BY_EXTENSION: Record = { export function languageOf(filename: string): ScanLanguage { if (filename.startsWith('.env') || filename.endsWith('.env')) return 'config'; - return LANGUAGE_BY_EXTENSION[extname(filename).toLowerCase()] ?? 'other'; + return LANGUAGE_BY_EXTENSION[extensionOf(filename).toLowerCase()] ?? 'other'; } /** Interpreters worth recognising, by the language their scripts are written in. */ @@ -78,10 +103,10 @@ export function languageOfShebang(firstLine: string): ScanLanguage | null { if (!match) return null; // `#!/usr/bin/env bash` names the interpreter in the argument, not the path. - const command = basename(match[1]!); + const command = baseNameOf(match[1]!); const args = match[2]?.trim().split(/\s+/) ?? []; const splitString = args[0] === '-S' || args[0] === '--split-string'; - const name = command === 'env' ? basename(args[splitString ? 1 : 0] ?? '') : command; + const name = command === 'env' ? baseNameOf(args[splitString ? 1 : 0] ?? '') : command; const exact = LANGUAGE_BY_INTERPRETER[name]; if (exact) return exact; @@ -91,38 +116,6 @@ export function languageOfShebang(firstLine: string): ScanLanguage | null { return (stripped ? LANGUAGE_BY_INTERPRETER[stripped] : undefined) ?? null; } -export interface ScanOptions { - /** Skip files larger than this. Defaults to 1 MiB. */ - maxFileBytes?: number; - /** Called once per file actually read, for progress reporting. */ - onFile?: (path: string) => void; - /** Restrict to these rule categories. Defaults to all. */ - categories?: readonly ScanFinding['category'][]; -} - -export interface ScanReport { - findings: ScanFinding[]; - filesScanned: number; - /** - * How many inline suppressions were honoured. Reported, never hidden: a - * scan that came back quiet because someone silenced forty rules is a - * different result from a scan that came back quiet. - */ - suppressed: number; - /** - * Directory that finding paths are relative to. Equal to the target for a - * directory scan, its parent for a single-file scan. SARIF URI resolution - * needs this — guessing it from the target is what produces file URIs that - * resolve to nothing. - */ - root: string; - /** - * Files matched by extension but unreadable. Reported rather than swallowed: - * a scan that could not read a file has not cleared it, and "0 findings" - * over an unread tree is the failure this scanner exists to avoid. - */ - unreadable: string[]; -} /** * Inline suppression, matching the convention `modules/code-scanner` already @@ -263,7 +256,7 @@ export function scanText( return findings; } -function scanManifest(relativePath: string, filename: string, text: string): ScanFinding[] { +export function scanManifest(relativePath: string, filename: string, text: string): ScanFinding[] { const manifestFindings = filename === 'package.json' ? scanPackageJson(text) @@ -286,188 +279,7 @@ function scanManifest(relativePath: string, filename: string, text: string): Sca })); } -export function scanPath(targetPath: string, options: ScanOptions = {}): ScanReport { - const maxFileBytes = options.maxFileBytes ?? 1024 * 1024; - const allowed = options.categories ? new Set(options.categories) : null; - const findings: ScanFinding[] = []; - const unreadable: string[] = []; - let filesScanned = 0; - let suppressed = 0; - - // A file target is not a degenerate directory target. `readdirSync` on a - // file throws ENOTDIR, which the walker below treats as an unreadable - // directory — so `threatcrush scan app.js` reported a clean scan of a file - // it never opened. Resolve the shape first, and walk only what is walkable. - const rootIsDirectory = (() => { - try { - return statSync(targetPath).isDirectory(); - } catch { - return true; - } - })(); - const walkRoot = rootIsDirectory ? targetPath : dirname(targetPath); - - const scanFile = (fullPath: string, filename: string): void => { - const relativePath = toRelative(walkRoot, fullPath); - const extension = extname(filename).toLowerCase(); - const isManifest = filename === 'package.json' || filename === 'requirements.txt'; - const scannable = SCAN_EXTENSIONS.has(extension) || filename.startsWith('.env'); - - // A file with no extension gets one question asked of it before being - // dismissed: does it start with a shebang? Executables are habitually named - // for what they do rather than what they are written in, and skipping them - // silently is how a repository whose only source file is `debtap` scans - // clean. Files that carry an unrecognised extension are still skipped — - // `.png` is not a script, and sniffing every one of them would mean reading - // the whole tree. - const mayDeclareInterpreter = !scannable && !isManifest && extension === ''; - - if (!scannable && !isManifest && !mayDeclareInterpreter) { - recordSensitiveFile(filename, relativePath, findings, []); - return; - } - - // Size-check and read through one descriptor. - // - // `statSync(path)` followed by `readFileSync(path)` is check-then-use: the - // path can be replaced between the two calls, so the size that was checked - // is not necessarily the size that gets read. Opening once and calling - // `fstatSync` on the descriptor removes the window — the descriptor refers - // to the same inode for both operations, whatever happens to the name. - // - // A scanner walking directories it does not control is exactly where this - // matters, and CWE-362 is a class this tool reports on. Worth getting - // right in its own walker. - let text: string; - let handle: number; - let declared: ScanLanguage | null = null; - try { - handle = openSync(fullPath, 'r'); - } catch { - unreadable.push(relativePath); - return; - } - - try { - if (fstatSync(handle).size > maxFileBytes) return; - - // Sniff the shebang from a short prefix rather than the whole file, so an - // extensionless blob — a checked-in binary, a data file — costs one small - // read instead of a megabyte decoded as UTF-8 and thrown away. - if (mayDeclareInterpreter) { - const prefix = Buffer.alloc(128); - const read = readSync(handle, prefix, 0, prefix.length, 0); - declared = languageOfShebang(prefix.subarray(0, read).toString('utf-8').split('\n', 1)[0] ?? ''); - if (!declared) return; - } - - text = readFileSync(handle, 'utf-8'); - } catch { - unreadable.push(relativePath); - return; - } finally { - try { - closeSync(handle); - } catch { - /* the descriptor is going away regardless */ - } - } - - filesScanned += 1; - options.onFile?.(relativePath); - suppressed += collectSuppressions(text.split('\n')).count; - - const fileFindings = [ - ...scanText(relativePath, text, declared ?? languageOf(filename)), - ...(isManifest ? scanManifest(relativePath, filename, text) : []), - ]; - - findings.push(...fileFindings); - recordSensitiveFile(filename, relativePath, findings, fileFindings); - }; - - const walk = (currentPath: string): void => { - let entries; - try { - entries = readdirSync(currentPath, { withFileTypes: true }); - } catch { - unreadable.push(toRelative(walkRoot, currentPath)); - return; - } - - for (const entry of entries) { - const fullPath = join(currentPath, entry.name); - - if (entry.isDirectory()) { - if (SKIP_DIRS.has(entry.name)) continue; - walk(fullPath); - continue; - } - if (!entry.isFile()) continue; - - scanFile(fullPath, entry.name); - } - }; - - if (rootIsDirectory) { - walk(targetPath); - } else { - scanFile(targetPath, basename(targetPath)); - } - - const filtered = allowed ? findings.filter((f) => allowed.has(f.category)) : findings; - filtered.sort( - (a, b) => - severityRank(b.severity) - severityRank(a.severity) || - a.file.localeCompare(b.file) || - a.line - b.line, - ); - - return { findings: filtered, filesScanned, unreadable, suppressed, root: walkRoot }; -} - -/** - * Report a file whose *name* is the finding — but only when its contents - * produced nothing. - * - * A `.env` full of detected credentials does not also need "this is a .env - * file" stapled to line 1. The filename finding exists for the case the - * content rules cannot cover: an env file whose values are shapes no vendor - * rule matches, which is still an env file that should not be committed. - */ -function recordSensitiveFile( - filename: string, - relativePath: string, - sink: ScanFinding[], - fileFindings: readonly ScanFinding[], -): void { - if (fileFindings.length > 0) return; - - for (const sensitive of SENSITIVE_FILES) { - const matches = filename === sensitive.pattern || filename.endsWith(sensitive.pattern); - if (!matches) continue; - sink.push({ - ruleId: 'sensitive-file-committed', - title: 'Sensitive file', - file: relativePath, - line: 1, - severity: sensitive.severity, - confidence: 'evidence', - message: sensitive.message, - consequence: 'Anything in this file is in every clone, fork and CI cache of the repository.', - cwe: 'CWE-538', - excerpt: '', - sensitive: true, - category: 'file', - }); - return; - } -} -function toRelative(base: string, target: string): string { - const rel = relative(base, target); - return (rel === '' ? target : rel).split(sep).join('/'); -} /** Highest severity present, or null for a clean scan. */ export function peakSeverity(findings: readonly ScanFinding[]): Severity | null { diff --git a/apps/cli/src/scan/types.ts b/packages/scan/src/types.ts similarity index 100% rename from apps/cli/src/scan/types.ts rename to packages/scan/src/types.ts diff --git a/packages/scan/tsconfig.json b/packages/scan/tsconfig.json new file mode 100644 index 0000000..50be3e1 --- /dev/null +++ b/packages/scan/tsconfig.json @@ -0,0 +1,17 @@ +{ + "compilerOptions": { + "target": "ES2022", + "module": "ESNext", + "moduleResolution": "bundler", + "esModuleInterop": true, + "allowSyntheticDefaultImports": true, + "strict": true, + "skipLibCheck": true, + "noEmit": true, + "declaration": false, + "resolveJsonModule": true, + "types": ["node"] + }, + "include": ["src/**/*"], + "exclude": ["node_modules"] +} diff --git a/packages/scan/vitest.config.ts b/packages/scan/vitest.config.ts new file mode 100644 index 0000000..0f63028 --- /dev/null +++ b/packages/scan/vitest.config.ts @@ -0,0 +1,8 @@ +import { defineConfig } from 'vitest/config'; + +export default defineConfig({ + test: { + include: ['src/**/__tests__/**/*.test.ts'], + environment: 'node', + }, +}); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 09cd25f..8e5cf54 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,6 +27,9 @@ importers: '@sentry/node': specifier: ^8.45.0 version: 8.55.1 + '@threatcrush/scan': + specifier: workspace:* + version: link:../../packages/scan better-sqlite3: specifier: ^11.7.0 version: 11.10.0 @@ -428,6 +431,18 @@ importers: specifier: ^3.0.0 version: 3.2.7(@types/debug@4.1.13)(@types/node@22.19.17)(jiti@2.6.1)(jsdom@29.1.1)(lightningcss@1.32.0)(terser@5.46.1)(yaml@2.8.3) + packages/scan: + devDependencies: + '@types/node': + specifier: ^22.10.1 + version: 22.19.17 + typescript: + specifier: ^5.6.3 + version: 5.9.3 + vitest: + specifier: ^3.2.4 + version: 3.2.7(@types/debug@4.1.13)(@types/node@22.19.17)(jiti@2.6.1)(jsdom@29.1.1)(lightningcss@1.32.0)(terser@5.46.1)(yaml@2.8.3) + packages: 7zip-bin@5.2.0: @@ -9113,7 +9128,7 @@ snapshots: '@babel/highlight@7.25.9': dependencies: - '@babel/helper-validator-identifier': 7.28.5 + '@babel/helper-validator-identifier': 7.29.7 chalk: 2.4.2 js-tokens: 4.0.0 picocolors: 1.1.1