map: trace imported helpers; protect: ship a real edge build - #120
Merged
Conversation
…otect.edge.js) Two follow-ups, both driven by tests that prove the behaviour rather than assert a proxy for it. 1. IMPORTED-HELPER TRACING (map). AI-generated apps put data access in a sibling module, so a handler's real sink lives one file away and the endpoint looked sink-free — nothing to correlate a CVE against. The extractor now follows ONE cross-file hop into a relative import (resolving extensionless, /index and the TS-ESM `./db.js` -> db.ts convention), plus one same-file hop inside that module, with a shared parse cache. Bare package specifiers are deliberately NOT followed: a dependency's internals are not this app's attack surface. Coverage notes state the hop limit. 2. A REAL EDGE BUILD. #117 made the Node imports dynamic, which keeps the module loadable off Node — but bundlers FOLLOW dynamic imports, so an edge build still failed to resolve `node:fs`/`node:path`, and the shipped dist/protect.js could NOT be bundled for Next edge middleware / Workers / Deno. (tsup also strips the `node:` prefix, and bare `fs` resolves nowhere on Workers while `node:fs` does under nodejs_compat.) So the source-level "no static node import" check was necessary but not sufficient. Adds scripts/build-edge.mjs: an esbuild build (platform browser) that replaces every Node-only module — and refresh-manifest, which pulls in the lockfile scanner — with a stub that rejects on import. The runtime already treats a failed `import('node:fs')` as "no filesystem here" and falls back to the memory/pluggable rule cache, so behaviour is preserved; the disk cache and manifest re-post are simply unavailable, which is correct on edge. Wired into `npm run build` and selected automatically via package.json exports conditions (workerd / worker / edge-light / deno / browser), with Node still getting the full build. tests/protect/edge-bundle.test.ts is the test that would have caught the original gap: it bundles the shipped artifact the way an edge bundler does (nothing external), asserts zero Node builtin references static OR dynamic, IMPORTS the bundle and verifies it still blocks an exploit with cacheDir set, and pins the exports condition order. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Adds an edge-safe protect build and augments map extractor with cross-file sink tracing. 🎯 Quality: 97% Elite · 📦 Size: Large — consider splitting if possible 📈 This month: Your 53rd PR — above team average · Averaging Excellent |
…s, Base44)
Platform function runtimes have no router and no route file: one handler per module,
invoked by the function's NAME. Without a recognizer, a Supabase Edge Functions or
Base44 backend-functions project mapped to NOTHING — no entry points, so no
reachability signal and nothing to correlate a vulnerability against.
Adds a `Deno.serve(handler)` / `serve(handler)` entry recognizer (entryKind
`edge-function`), derives the deployed function name and route from the conventional
location (`supabase/functions/<name>/index.ts`, `functions/<name>.ts` → `/<name>`),
extends the textual pre-filter so those files are parsed at all, and detects the
project shape (`supabase-functions` / `deno-functions`) even when there is no
package.json — which is normal for a Deno project.
The existing request-read extraction already covers the idiomatic
`const { a, b } = await req.json()`, so these endpoints get real inputs, sinks and
PROVEN flows: the fixture's `hook -> fetch` (the classic SSRF shape) comes out as a
precise flow, which is exactly what a rule needs to pin.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
/review |
…and locatable
Round-2 review fixes. Both are cases where the previous implementation could make a
claim it hadn't earned.
1. FALSE `precise` FLOWS (high). linkFlows concatenated all argument source text and
then checked, independently, that (a) the input's leaf name appeared anywhere and
(b) any tainted root appeared anywhere. So:
const { title } = await req.json();
db.from("items").insert({ title: "system", owner: req.user.id });
was reported `title -> insert [precise]` — `title` matched a property KEY and `req`
matched a different value. Since a consumer may PIN A RULE on `precise`, that is the
exact false positive the designation exists to avoid.
Replaced with AST evidence: collect leaves genuinely READ from a tainted source
(`data.title`, `req.body.title`, `{ title }` shorthand, `fn(title)`, `x[\"title\"]`),
explicitly excluding property keys, member names and binding names; a flow is
`precise` only if the input's leaf is among them, else `heuristic`. Evidence is
gathered from the enclosing statement so a fluent chain
(`.update({…}).eq('id', data.id)`) counts as one operation.
2. IMPORTED HELPERS (medium).
- The resolver did not enforce the project boundary the walker enforces, so
`import '../../other-repo/db'` (or a symlink) could pull an unrelated codebase into
this app's attack surface. It now rejects anything whose realpath leaves the project
unless --follow-symlinks.
- An imported sink kept the HELPER's line number while the endpoint reported the
handler's file, so the coordinate pointed at the wrong file. Sinks reached through an
import now carry their own `file`; flow linking also refuses to call such a sink
`precise`, since its call site isn't visible in the handler.
- `import { saveOrder as write } from './db'` looked up `write` in the target module and
missed the helper. Bindings now track the exported name behind an alias.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
daniloradovic
approved these changes
Aug 13, 2026
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.
Two follow-ups from the review backlog, both driven by tests that prove behaviour instead of asserting a proxy for it.
1. Imported-helper tracing (
map)AI-generated apps put data access in a sibling module (
import { saveOrder } from './lib/db'), so a handler's real sink lives one file away — the endpoint looked sink-free, leaving nothing to correlate a CVE against.The extractor now follows one cross-file hop into a relative import — resolving extensionless paths,
/index, and the TS-ESM./db.js→db.tsconvention — plus one same-file hop inside that module, behind a shared parse cache. Bare package specifiers are deliberately not followed: a dependency's internals aren't this app's attack surface. The hop limit is stated incoverage.notes.2. A real edge build — and why #117 wasn't enough
#117 made the Node imports dynamic, which keeps the module loadable off Node. But bundlers follow dynamic imports, so an edge build still failed on
node:fs/node:path— the shippeddist/protect.jscould not be bundled for Next edge middleware / Workers / Deno. Two aggravating details found while testing:tsupstrips thenode:prefix (node:fs→fs), and barefsresolves nowhere on Workers whilenode:fsresolves undernodejs_compat.refresh-manifestdynamic import drags in the whole lockfile scanner (node:fs/promises).So the source-level "no static Node import" check (#117) was necessary but not sufficient. This adds
scripts/build-edge.mjs— an esbuild build (platform: 'browser') that replaces every Node-only module, andrefresh-manifest, with a stub that rejects on import. The runtime already treats a failedawait import('node:fs')as "no filesystem on this runtime" and falls back to the memory / pluggable (ruleCache) tiers, so behaviour is preserved — the disk cache and manifest re-post are simply unavailable, which is correct on edge.Wired into
npm run build, shipped in the tarball, and selected automatically throughexportsconditions:The test that would have caught it
tests/protect/edge-bundle.test.tsbundles the shipped artifact the way an edge bundler does (nothing external), asserts zero Node builtin references — static or dynamic — then imports the bundle and verifies it still blocks an exploit withcacheDirset (the disk tier failing open), and pins theexportscondition order so an edge condition can't fall behindimport.761 tests green; typecheck + build clean.