Enforce the two-entry .tcade contract in the package reader

[nicodes]

Refs aviorstudio/termcade-be#42

What

docs/packaging.md has always said a .tcade is exactly termcade.toml and game.wasm at its root. The reader only looked those two names up and silently ignored everything else:

• extra files passed through unread,
• duplicate entries (zip permits repeated names) were resolved first-match-wins,
• nested lookalikes like assets/game.wasm and directory entries were ignored,
• an encrypted entry failed later with a misleading error (Go's reader cannot decrypt one; empirically ReadAll yields empty content with a nil error, surfacing as game.wasm is not a wasm module).

readPackage now classifies every entry in the central directory before reading anything: the two root names exactly once each, nested and unexpected entries rejected by name, encrypted entries (general-purpose flag bit 0) rejected explicitly, missing entries reported as before. The existing security bounds are retained: 1 MiB manifest limit, 64 MiB wasm limit, wasm magic check.

This is the public manifest module the registry validates uploads against, so the arcade and the marketplace keep deciding by one piece of code.

Tests

New TestPackageEntryContract builds fixtures with raw zip construction (duplicates via Writer.Create, encryption flag via CreateRaw) that WritePackage cannot produce:

• extra root entry, duplicate manifest, duplicate wasm, nested lookalike manifest, nested lookalike wasm, directory entry, encrypted entry, missing wasm, empty archive — each rejected with a stable message substring;
• valid two-entry packages accepted in either entry order.

Gate proven fallible: run against the previous implementation (stash of manifest/package.go), the third-root-entry fixture and six sibling cases fail — e.g. TestPackageEntryContract/extra_root_entry: accepted, want rejection containing "unexpected package entry \"README.md\"". Restored code passes all ten subtests. The two missing-entry cases already failed before this change (pre-existing behavior, retained).

Verification (all from this worktree, go1.26.5)

• gofmt -l . — clean
• go vet ./... && go vet ./sdk/... — clean
• go test -race -count=1 -timeout 10m ./... ./sdk/... — all pass, incl. internal/plugin wasm E2E (87.9s, guests built from the working tree; E2E skips only under -short, not passed here)
• GOWORK=off go build ./... && GOWORK=off go vet ./... — clean (consumer resolution)
• GOWORK=off go test -count=1 ./manifest/ ./internal/starter/ and GOWORK=off go test -count=1 -timeout 10m ./internal/plugin/ — pass (host + manifest as an external consumer resolves them)
• Bundled starter packs verified two-entry via unzip -l; internal/starter reads them through ReadPackage and passes.

After merge

The registry (aviorstudio/termcade-be#43) consumes github.com/aviorstudio/termcade/manifest as a versioned module. After this merges, cut the normal arcade release (Release workflow, patch bump → v0.0.8; v0.0.7 is the current tag) so the strict validator is published through the usual gates. No tag created here.

[nicodes]

Review 1

P1 — Directory-marked required entries are still accepted

manifest/package.go:82-93

rootEntries only identifies directories by / in the entry name. Go’s ZIP semantics also represent directories through external file attributes (f.FileInfo().IsDir()), which can mark an entry named exactly termcade.toml or game.wasm as a directory without a trailing slash. A crafted archive with the two allowed names, valid payloads, and (for example) game.wasm marked os.ModeDir passes this check and is accepted, contradicting the documented/issue requirement that directory entries be rejected.

Test/coverage residuals

manifest/manifest_test.go:174-177 tests only a trailing-slash directory name. Add coverage for required-name entries carrying directory external attributes (and ideally other non-regular modes). CI ci was reported successful; I did not execute tests.

Method: reviewed only b1099aeb40fa1ad44983d9c5a261d7d887308697...fa295a375481cc243298525023180196b8427721 and checked the ZIP behavior against Go’s versioned archive/zip API/source semantics.

Verdict: NEEDS ATTENTION

[nicodes]

Review 1 reply

Fixed in f7b53b9. Go can mark directories through external attributes without a trailing slash, so both required entries now must satisfy f.FileInfo().Mode().IsRegular(). This rejects directory, symlink, device, and other non-regular modes because the two-file package contract assigns them no meaning.

Reachable cases now cover termcade.toml and game.wasm marked fs.ModeDir without trailing slashes plus game.wasm marked fs.ModeSymlink. Against the pre-fix head all three were accepted; the exact failure included accepted, want rejection containing "\"termcade.toml\" is not a regular file". Existing size limits and messages remain intact, documentation now states regular-file enforcement, the full race/E2E and GOWORK=off gates pass, and head CI is green.

Residual coverage: unexpected and nested names are rejected before their modes matter, so type assertions intentionally bind only to the two accepted names. Encryption is checked first and reports encryption if combined with a non-regular mode. Corrupt-CRC and oversized entry behavior remains in the existing read/limit paths.

[nicodes]

Review 2

No findings.

Review 1 disposition verified: manifest/package.go:84-86 rejects either required entry unless FileInfo().Mode().IsRegular() succeeds; manifest/manifest_test.go:190-203 covers attribute-marked directories for both required names and a symlink. This closes the reported acceptance path.

Method: static review of the exact pinned range b1099aeb40fa1ad44983d9c5a261d7d887308697...f7b53b9e3406828add7363ae314f193f30713195 (3 files, 2 commits), including Go 1.26.2 archive/zip API semantics. CI success was reported; tests were not independently executed. Residual gap: fixtures exercise ReadPackage; OpenPackage shares the same readPackage path.

Verdict: APPROVE

[nicodes]

Review 2 reply

No findings require changes. The residual gap is explicitly dispositioned: OpenPackage and ReadPackage both enter the same readPackage function before the new rootEntries check, so duplicating every malformed ZIP fixture through both public wrappers would not exercise a distinct assertion. Head CI is green.

Disposition: Review 2 is approving.