Commit Graph

4 Commits

Author SHA1 Message Date
FilipBl4gojevic
a56e70deae fix(signer): harden the boundary check's globalThis blind spot, and stop overstating it
The review verified six obfuscations that bypass boundary.test.ts's ambient-
global and forbidden-import checks: globalThis['fetch'](...), an alias
(const f = fetch; f(...)), an aliased bracket reference, the comma-operator
trick ((0, globalThis.fetch)(...)), and two dynamic import()s with a
concatenated specifier (import('node:' + 'dns'), import('child_' +
'process')). README.md called the check "the package's central security
guarantee" and boundary.test.ts's header claimed the signer "CANNOT reach
the network -- not that it chooses not to." Neither line was true of what a
text scan can actually promise.

Hardened cheaply, within reach of a text scan:
- FORBIDDEN_IMPORTS gains node:dns, node:dns/promises, node:http2,
  node:worker_threads, node:vm, each with a one-line rationale.
- AMBIENT_NETWORK_GLOBALS gains globalThisQualifiedAccess(name), matching a
  global reached via globalThis in either dot or bracket ("computed member
  access") notation, plus navigator['sendBeacon'] bracket notation.

Verified empirically against the real hardened check (temporary probe
files under src/, one per evasion, deleted immediately after -- see the
task report for the full per-file breakdown): globalThis['fetch'](...),
the aliased-bracket-reference variant, and the comma-operator
globalThis.fetch trick are now all caught. The plain-identifier alias and
both concatenated dynamic imports still pass -- those need real data-flow
tracking or evaluating an arbitrary runtime expression, neither of which a
text scan can do, and no attempt was made to chase them.

Reworded both the README section and boundary.test.ts's header comment to
say what the check actually verifies: today's shipped src/ has no
accidental or careless network capability. Deliberate obfuscation is named
as a code-review responsibility, not something this file promises to catch.

Final whole-branch review, Fix 4 of 4.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-07-28 18:33:14 +02:00
FilipBl4gojevic
de6ae72db9 test(signer): close the shell-out bypass and three evasions in the boundary test
Review found node:child_process had zero coverage across all four assertions:
execSync('curl ...') would have passed every one. Forbidden now, and verified
to fail when injected.

Also closed: backtick-quoted specifiers evaded the substring and regex scans;
sourceFiles() only walked .ts so a stray .js was invisible; the dependency
check ignored optional/peer deps and named only two packages.
2026-07-28 11:30:10 +02:00
FilipBl4gojevic
34b2d7fa18 test(signer): make the no-network boundary test able to actually fail
Two holes: the shared-dependency assertion was a tautology that held for any
manifest, and ambient globals (fetch, WebSocket) need no import so the import
scan was blind to them. Both now enforced, and both verified to fail when a
violation is injected.
2026-07-28 11:13:15 +02:00
FilipBl4gojevic
46e814b7ae feat(signer): scaffold the package and enforce the no-network boundary
The signer's security claim is that it cannot reach the network, not that it
chooses not to. test/boundary.test.ts enforces that mechanically over both the
import graph and the manifest.
2026-07-28 11:05:25 +02:00