Skip to content

Complete upload-adjacent readiness work: transactions, cross-file authorization, canonical installation#54

Open
is-bo wants to merge 11 commits into
mainfrom
fix/complete-external-readiness
Open

Complete upload-adjacent readiness work: transactions, cross-file authorization, canonical installation#54
is-bo wants to merge 11 commits into
mainfrom
fix/complete-external-readiness

Conversation

@is-bo

@is-bo is-bo commented Jul 27, 2026

Copy link
Copy Markdown
Owner

Completes three of the five items left unfinished after #53, and fixes several defects found while doing so. This is partial — two items are not done and are listed explicitly below. Do not merge on the assumption that the completion work is finished.

Baseline 3f73177 · candidate 1f7342b · 9 commits · package version unchanged at 0.1.0, no tag, no release.

What is complete

Missing-transaction analysis (new)

There was no transaction detection in the CLI at all. This adds a deterministic analyzer that flags multi-step write workflows where a partial failure would violate an evidenced consistency invariant, and wires it into the per-file pass so the rules actually execute rather than being declared only.

Relatedness is structural: writes must share an entity identifier, a foreign key, or dataflow before they are grouped, so a function containing several independent writes stays clean. Boundaries resolve through Prisma, Knex, Sequelize, TypeORM, Drizzle, Mongo sessions, raw BEGIN/COMMIT, local aliases, and one level of wrapper delegation. Anything unresolvable becomes NOT_VERIFIED.

Two defects were found by the new regression matrix and fixed:

  • isKnownTransactionChain branched on segments.length === 1, which routed every single-hop member call — prisma.$transaction, knex.transaction — down an alias-only path where a vendor API could never match. Correctly wrapped transactions were being reported.
  • hasDataAccessReceiver gated ambiguous verbs behind a receiver-name check, so writes on a handle from a custom wrapper matched no known data-access name and were not detected as writes at all. An unresolved boundary produced no finding in either direction — silently safe, which the contract forbids.

Cross-file authorization middleware

classifyRouteGuards accepted any unresolved middleware whose identifier began with require/ensure/authorize/etc. as a proven guard. Since an imported symbol never resolved to a local body, every cross-file guard took that path: an imported requireAdmin whose body only calls next() suppressed the route finding entirely.

Classification now goes through a corpus-wide resolver that reads the body an import actually names — following relative imports, renamed and default exports, barrel re-exports, and factories, under explicit hop, file, and cycle budgets, reusing scope.ts for module resolution rather than adding a second resolver. The three in-file helpers are deleted, so there is one implementation.

Verified on cross-file fixtures: an imported requireAdmin that never denies now fails; tollbooth that returns 403 is clean; barrel re-export resolves; external package, cycle, and dynamic import are all NOT_VERIFIED rather than proven.

Canonical installation layout

Six host roots each carried a full copy of every managed file. They now carry a SKILL.md adapter that preserves host discovery frontmatter and names the canonical playbook by relative path. No symlinks, so Windows and paths containing spaces behave identically. Codex remains a documented exception: it reads agents/openai.yaml with ordinary tooling, so agents/ and assets/ are still copied verbatim into .agents only.

Files Bytes
Before 804 6,704,066
After (134 canonical + 286 host) 420 2,229,170
Reduction −47.8% −66.7%

The layout would have shipped broken. installer.ts reads bundled content from PACKAGE_ROOT/.fullstack-forge, but that directory was missing from the package files allowlist, so the entire managed payload was excluded from the tarball. Fresh install, upgrade, and offline install all failed with ENOENT on a packed archive. In-repo tests could not catch this because they read the working tree.

Verified on Windows at a path containing a space:

  • smoke:install — 46 skills, automatic activation, 0 symlinks, clean uninstall
  • smoke:upgrade — migration from the previous full-copy layout, doctor ready, 46 per root, 0 symlinks, clean uninstall
  • offline:install — six roots at 46 skills, cache-only npm, unreachable registry, 0 symlinks, clean uninstall

One upload false positive closed

FF-UPLOAD-MIME-001 fired whenever a file mentioned a content-type token and did not literally contain magic|signature|fileTypeFromBuffer|decode|sniff. Validation is routinely factored into a shared helper whose body the in-file regex cannot read, so hardened upload paths were published as confident HIGH failures purely because their validation lived in another module.

When the upload payload is passed to an imported function, the finding is now NOT_VERIFIED and its evidence names the delegate. Detection is structural — import bindings plus an AST walk for a call receiving the payload — not a name list. Trusting client MIME with no validation anywhere still fails; proven in-file signature validation is still clean.

This closes the false-positive class. It is not upload analyzer completion — see below.

What is NOT done

  • Upload analyzer coverage (0A). The three target rule families were not rewritten structurally, no new fixtures were added for them, and non-multer coverage is unchanged.
  • Playbook deduplication (0C). Not started; boilerplate remains at its current level and is not measured here.
  • Independent verification, external benchmarking, and release-readiness scoring. Not performed.
  • npm run test:coverage and npm audit were not run. Only Node 24.14.1 was exercised — not 20 or 22 — and no CI evidence was collected for the nine-job OS/Node matrix.

Known defects found but not fixed

  • Upload analysis is multer-only. The analyzer is gated on upload.(any|array|fields|single), so presigned S3/GCS, busboy, formidable, and Next.js route handlers receive no upload analysis. Their "clean" result is indistinguishable from genuinely hardened code, which is the more dangerous failure mode.
  • FF-AUTHZ-OBJECT-001 fires behind proven guards. Any delete({ where: { id } }) reports HIGH even when a proven admin guard sits in front. Defensible in principle — role access is not per-object ownership — but on real repositories this has the shape of a systemic HIGH false-positive class. Pre-existing; flagged rather than changed, since suppressing it could hide genuine BOLA bugs.

Validation

npm run check passes end to end on Node 24.14.1 / Windows: format, lint, typecheck, 867 tests (866 pass, 1 skipped, 0 fail), all validators, packaging (9 archives, 1083 entries), and secret scan (1180 files, 0 findings). Baseline was 845 tests; the 22 added are the transaction, guard-resolution, and upload-delegation matrices.

Three test files and three smoke scripts asserted the old full-copy layout. They were rewritten to assert the property that matters — canonical content exists, adapters point at it, unselected hosts stay untouched — and are stricter than before, not weaker. transactions.ts, guard-resolution.ts, and managed-layout.ts had never been linted (they were untracked); doing so surfaced 8 real defects, all fixed at source.

🤖 Generated with Claude Code

is-bo and others added 11 commits July 27, 2026 02:03
Detects multi-step write workflows where a partial failure would violate an
evidenced consistency invariant, and wires the family into the js-ts-boundaries
per-file pass so the rules are reachable rather than declared-only.

Relatedness is structural, not positional: writes must share an entity
identifier, a foreign key, or dataflow before they are grouped, so ordinary
functions containing several independent writes stay clean. Boundaries resolve
through Prisma, Knex, Sequelize, TypeORM, Drizzle, Mongo sessions, raw
BEGIN/COMMIT, local aliases, and one level of wrapper delegation. An
abstraction the analyzer cannot resolve becomes NOT_VERIFIED rather than a
silent pass or a confident failure.

Two defects found by the new regression matrix and fixed here:

- isKnownTransactionChain branched on segments.length === 1, which routed every
  single-hop member call (prisma.$transaction, knex.transaction) down the
  alias-only path where a vendor API could never match. Correctly wrapped
  transactions were reported as unresolved. A bare call is now distinguished
  from a member call by comparing the root to its own first segment.

- hasDataAccessReceiver gated ambiguous verbs behind a receiver-name check, so
  writes performed on a handle from a custom wrapper matched no known
  data-access name and were not detected as writes at all. An unresolved
  boundary therefore produced no finding in either direction. Receivers that
  are parameters of a callback passed to a call now qualify; ordinary
  declaration parameters still do not, so route handler req/res cannot match.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces full per-host duplication with one canonical managed copy under
.fullstack-forge/skills plus small per-host adapter files. Six host roots
previously carried a complete copy of every skill; they now carry a SKILL.md
adapter that preserves the frontmatter each host needs for discovery and names
the canonical playbook by relative path. No symlinks are involved, so the
layout behaves identically on Windows and on paths containing spaces.

Generation reports 133 canonical files and 276 adapters across 6 host roots,
and the tree shrinks by roughly 37.6k lines.

Codex is a documented exception: it reads .agents/skills/<skill>/agents/openai.yaml
with ordinary tooling rather than by following a prose pointer, and that file
declares its icon relative to itself, so agents/ and assets/ are still copied
verbatim into .agents roots only.

InstallFile gains kind and platforms so uninstall and doctor can distinguish
canonical content, adapters, and user files; the manifest schema accepts 1 or 2
to allow migration from the previous full-copy layout.

Three tests encoded the old layout by asserting physical duplication in every
host root. They now assert the property that matters -- canonical content
exists, adapters point at it, and unselected hosts stay untouched -- and are
stricter than before: the installer test additionally asserts that a
claude,cursor selector never writes .agents, .gemini, .github, or .windsurf.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ts name

classifyRouteGuards accepted any unresolved middleware whose identifier began
with require/ensure/assert/check/verify/enforce/guard/can/is/has/only/restrict/
protect/authorize/authenticate as a proven guard. Because an imported symbol
never resolved to a local body, every cross-file guard took that path: an
imported requireAdmin whose body only calls next() suppressed the route finding
entirely. That is the opposite of fail-closed and is not safe for scanning
external repositories.

Route classification is now delegated to a corpus-wide resolver that reads the
body an import actually names. It follows local relative imports, renamed and
default exports, barrel re-exports, and middleware factories, under explicit
hop, file, and cycle budgets, and reuses scope.ts for module resolution rather
than introducing a second resolver.

Outcomes are now honest: a structurally proven guard is clean, a resolved body
that never denies is a failure, and anything unresolvable -- an external
package, a cycle, a dynamic import -- is NOT_VERIFIED rather than proven or
confidently failed. The resolver's reason string is published as evidence so a
reader can see which hops were taken.

The in-file isConventionalGuardName, resolveGuardFunction, and
functionDeniesAuthorization helpers are removed; guard-resolution.ts is now the
single implementation, so there is no second alias engine to keep in step.

Adds an 11-case matrix over direct, renamed, default, barrel, two-hop, factory,
cyclic, external-package, dynamic-import, misleading-name, and
honest-but-oddly-named middleware.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
transactions.ts, guard-resolution.ts, managed-layout.ts, and the new generator
library were added as untracked files, so npm run check never linted them. With
them tracked, eslint and prettier report real defects:

- useless escapes in three raw-SQL character classes
- a provably unreachable undefined check in callChain
- an unnecessary type assertion in the installer manifest record
- two literal U+FEFF characters embedded in regular expressions, now written as
  \uFEFF escapes
- Buffer used in an .mjs generator without importing node:buffer

.fullstack-forge/ joins the other generated roots in .prettierignore. The
canonical tree is generator output like .claude/ and .agents/, so formatting it
by hand would be reverted on the next npm run generate.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The repository-identity gate asserts that generated skill content carries the
public repository URL. It scanned only the six host roots, which under the
previous layout each held a full copy of every managed file. Those roots now
hold thin adapters, and the files carrying the reference -- the bundled JSON
schemas -- live once under .fullstack-forge/skills/, so the gate failed even
though the reference is still shipped.

The canonical root joins the scanned prefixes. The assertion is unchanged:
generated managed content must remain attributable to the public repository.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…apters

The installer reads bundled managed content from PACKAGE_ROOT/.fullstack-forge,
but that directory was never added to the package files allowlist, so it was
absent from the published tarball. Installing from a packed archive failed
outright with ENOENT on the canonical skills directory: fresh install, upgrade,
and offline install were all broken. .fullstack-forge/ now ships.

The three installation smoke scripts also asserted the previous layout by
reading playbook prose out of host skill files, which are now adapters, and by
expecting agents/openai.yaml under every host rather than under Codex and the
canonical root. They now assert both halves of the contract: each host adapter
names its canonical target, and the canonical file carries the real content. A
broken pointer or missing managed content still fails.

Verified end to end on Windows with a path containing a space:

  smoke:install    46 skills, automatic activation on, 0 symlinks, clean uninstall
  smoke:upgrade    migration from the previous full-copy layout, doctor ready,
                   46 skills per root, 0 symlinks, clean uninstall
  offline:install  all six roots at 46 skills, cache-only npm, unreachable
                   registry, 0 symlinks, clean uninstall

Measured deduplication across the generated roots:

  before  804 files  6,704,066 bytes
  after   420 files  2,229,170 bytes  (134 canonical + 286 host)
  saving  47.8% files, 66.7% bytes

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reports what was actually implemented and validated, and states plainly what
was not: upload analysis and playbook deduplication are unimplemented, and no
independent verification, external benchmark, or release scoring was performed.

Includes the command ledger with exit codes, the measured installation
deduplication, the two transaction defects and the imported-guard defect found
and fixed, and the known false-positive classes left open.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…other file

FF-UPLOAD-MIME-001 fired whenever a file mentioned a content-type token and did
not literally contain magic/signature/fileTypeFromBuffer/decode/sniff. Content
validation is routinely factored into a shared helper, and the in-file regex
cannot read that body, so hardened upload paths were published as confident
HIGH failures purely because their validation lived in another module.

When the upload payload is passed to an imported function, the finding is now
NOT_VERIFIED and its evidence names the delegate that could not be resolved.
Detection is structural -- import bindings plus an AST walk for a call that
receives the payload -- not a name list.

Trusting client MIME with no validation anywhere still fails, and proven
in-file signature validation is still clean, so no true positive regresses.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Updates the candidate SHA, commit count, and test totals, and states precisely
what the upload work did and did not cover: the cross-file false-positive class
is closed, but the three target rule families were not rewritten and non-multer
upload paths remain unanalysed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Establishes an honest, reproducible baseline for the deduplication work rather
than carrying an unverified figure. Units are semantic (frontmatter entry,
heading, list item, paragraph) and normalized, so re-wrapping prose cannot move
the score, and a wrapping-independent shingle cross-check is reported alongside.

Measured on the 42 canonical specialist playbooks at this commit:

  primary (literal units)   32.89%   25981 tokens, 8545 shared
  masked (module name/title) 42.19%
  shingle-8 cross-check      27.31%
  worst file                 37.80%  forge-realtime

The target is below 25%, so the deduplication itself is still outstanding. The
script only measures; no content has been moved.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the unmeasured note with the real figures now that the measurement
script has been reviewed and run: 32.89% primary against a below-25% target,
with the method and corpus stated. The deduplication itself is still not done,
so no before/after pair is claimed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant