diff --git a/docs/packaging.md b/docs/packaging.md index ecaa3b4..add0446 100644 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -42,8 +42,14 @@ mygame.tcade └── game.wasm ``` -`termcade dev build [dir]` produces one at `/build/.tcade` (it compiles -`./cmd/wasm`, validates the module's exports, and zips it with the manifest). +"Exactly" is enforced, not implied: a package carrying anything else — an +extra file, a second `termcade.toml` or `game.wasm` (zip permits duplicate +names), a nested or directory entry such as `assets/game.wasm`, a required +name marked as anything but a regular file (zip also marks directories by +external attributes, with no trailing slash needed), or an encrypted entry — +is rejected outright rather than having the extra bytes silently ignored. `termcade dev build [dir]` produces one at +`/build/.tcade` (it compiles `./cmd/wasm`, validates the module's +exports, and zips it with the manifest). ## Installing diff --git a/manifest/manifest_test.go b/manifest/manifest_test.go index 4184bfc..08cd1ca 100644 --- a/manifest/manifest_test.go +++ b/manifest/manifest_test.go @@ -1,6 +1,9 @@ package manifest import ( + "archive/zip" + "bytes" + "io/fs" "os" "path/filepath" "strings" @@ -103,6 +106,135 @@ func TestPackageRoundTrip(t *testing.T) { } } +var goodWasm = []byte{0x00, 0x61, 0x73, 0x6d, 0x01, 0x00, 0x00, 0x00} + +type zipEntry struct { + name string + body []byte + encrypted bool // sets general-purpose flag bit 0 without encrypting + mode fs.FileMode // external attributes, e.g. fs.ModeDir without a "/" name +} + +// rawZip builds a zip in memory with exactly the entries given, in order — +// including duplicate names (legal in zip, rejected by the package contract), +// encrypted flags and attribute-marked directories, none of which +// WritePackage can produce. +func rawZip(t *testing.T, entries ...zipEntry) []byte { + t.Helper() + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + for _, e := range entries { + var w interface{ Write([]byte) (int, error) } + var err error + switch { + case e.encrypted: + fh := &zip.FileHeader{Name: e.name, Method: zip.Store} + fh.Flags |= 0x1 + w, err = zw.CreateRaw(fh) + case e.mode != 0: + fh := &zip.FileHeader{Name: e.name, Method: zip.Deflate} + fh.SetMode(e.mode) + w, err = zw.CreateHeader(fh) + default: + w, err = zw.Create(e.name) + } + if err != nil { + t.Fatal(err) + } + if _, err := w.Write(e.body); err != nil { + t.Fatal(err) + } + } + if err := zw.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +// A .tcade holds exactly one termcade.toml and one game.wasm at its root and +// nothing else; every deviation below must be rejected by name, never +// silently ignored. +func TestPackageEntryContract(t *testing.T) { + cases := map[string]struct { + entries []zipEntry + want string // substring of the rejection message + }{ + "extra root entry": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}, {"README.md", []byte("hi"), false, 0}}, + want: `unexpected package entry "README.md"`, + }, + "duplicate manifest": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}, {FileName, []byte(goodTOML), false, 0}}, + want: "package has more than one " + FileName, + }, + "duplicate wasm": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}, {WasmName, goodWasm, false, 0}}, + want: "package has more than one " + WasmName, + }, + "nested lookalike manifest": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}, {"sub/" + FileName, []byte(goodTOML), false, 0}}, + want: `"sub/` + FileName + `" is not at the root`, + }, + "nested lookalike wasm": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}, {"sub/" + WasmName, goodWasm, false, 0}}, + want: `"sub/` + WasmName + `" is not at the root`, + }, + "directory entry": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}, {"sub/", nil, false, 0}}, + want: `"sub/" is not at the root`, + }, + // A directory is marked by external attributes too, not only by a + // trailing slash: a required name carrying ModeDir (or any other + // non-regular mode) is not a file, whatever its name says. + "manifest marked as directory": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, fs.ModeDir | 0o755}, {WasmName, goodWasm, false, 0}}, + want: `"` + FileName + `" is not a regular file`, + }, + "wasm marked as directory": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, fs.ModeDir | 0o755}}, + want: `"` + WasmName + `" is not a regular file`, + }, + "wasm marked as symlink": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, fs.ModeSymlink | 0o777}}, + want: `"` + WasmName + `" is not a regular file`, + }, + "encrypted entry": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, true, 0}}, + want: `"` + WasmName + `" is encrypted`, + }, + "missing wasm": { + entries: []zipEntry{{FileName, []byte(goodTOML), false, 0}}, + want: "package has no " + WasmName + " at its root", + }, + "empty archive": { + entries: nil, + want: "package has no " + FileName + " at its root", + }, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + _, err := ReadPackage(rawZip(t, tc.entries...)) + if err == nil { + t.Fatalf("accepted, want rejection containing %q", tc.want) + } + if !strings.Contains(err.Error(), tc.want) { + t.Errorf("error %q does not contain %q", err, tc.want) + } + }) + } + + // The contract's other side: exactly the two right entries pass it, in + // either order. + for _, entries := range [][]zipEntry{ + {{FileName, []byte(goodTOML), false, 0}, {WasmName, goodWasm, false, 0}}, + {{WasmName, goodWasm, false, 0}, {FileName, []byte(goodTOML), false, 0}}, + } { + if _, err := ReadPackage(rawZip(t, entries...)); err != nil { + t.Errorf("valid package %v rejected: %v", entries[0].name, err) + } + } +} + func TestPackageRejects(t *testing.T) { dir := t.TempDir() diff --git a/manifest/package.go b/manifest/package.go index e3f4468..a3d7022 100644 --- a/manifest/package.go +++ b/manifest/package.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "strings" "github.com/BurntSushi/toml" ) @@ -44,7 +45,11 @@ func ReadPackage(raw []byte) (*Package, error) { } func readPackage(zr *zip.Reader) (*Package, error) { - manifestRaw, err := zipFile(zr, FileName, 1<<20) + manifestF, wasmF, err := rootEntries(zr) + if err != nil { + return nil, err + } + manifestRaw, err := readEntry(manifestF, 1<<20) if err != nil { return nil, err } @@ -52,7 +57,7 @@ func readPackage(zr *zip.Reader) (*Package, error) { if err != nil { return nil, err } - wasm, err := zipFile(zr, WasmName, maxWasmSize) + wasm, err := readEntry(wasmF, maxWasmSize) if err != nil { return nil, err } @@ -62,26 +67,71 @@ func readPackage(zr *zip.Reader) (*Package, error) { return &Package{Manifest: m, Wasm: wasm}, nil } -func zipFile(zr *zip.Reader, name string, limit int64) ([]byte, error) { +// rootEntries enforces the package contract: exactly one termcade.toml and +// one game.wasm at the zip's root, and nothing else. A zip's central +// directory may carry duplicate names, nested paths and directory entries, +// and a .tcade comes from strangers, so each violation is its own explicit +// rejection rather than a silently ignored entry. +func rootEntries(zr *zip.Reader) (manifestF, wasmF *zip.File, err error) { for _, f := range zr.File { - if f.Name != name { - continue - } - rc, err := f.Open() - if err != nil { - return nil, fmt.Errorf("reading %s: %w", name, err) + // zip's general-purpose flag bit 0 marks an encrypted entry. The + // reader cannot decrypt one, so reject it by name instead of + // letting it fail later as a read error or an empty file. + if f.Flags&0x1 != 0 { + return nil, nil, fmt.Errorf("package entry %q is encrypted", f.Name) } - defer rc.Close() - raw, err := io.ReadAll(io.LimitReader(rc, limit+1)) - if err != nil { - return nil, fmt.Errorf("reading %s: %w", name, err) + // A directory is also marked by external attributes, with no + // trailing slash required — so a name match is not enough. The two + // required entries must be regular files (FileInfo semantics, which + // cover the MS-DOS directory bit and Unix mode bits alike); any + // other type — directory, symlink, device — has no meaning in a + // two-file package. + if f.Name == FileName || f.Name == WasmName { + if !f.FileInfo().Mode().IsRegular() { + return nil, nil, fmt.Errorf("package entry %q is not a regular file", f.Name) + } } - if int64(len(raw)) > limit { - return nil, fmt.Errorf("%s exceeds the %d byte limit", name, limit) + switch f.Name { + case FileName: + if manifestF != nil { + return nil, nil, fmt.Errorf("package has more than one %s", FileName) + } + manifestF = f + case WasmName: + if wasmF != nil { + return nil, nil, fmt.Errorf("package has more than one %s", WasmName) + } + wasmF = f + default: + if strings.Contains(f.Name, "/") { + return nil, nil, fmt.Errorf("package entry %q is not at the root: a .tcade contains only %s and %s", f.Name, FileName, WasmName) + } + return nil, nil, fmt.Errorf("unexpected package entry %q: a .tcade contains only %s and %s", f.Name, FileName, WasmName) } - return raw, nil } - return nil, fmt.Errorf("package has no %s at its root", name) + if manifestF == nil { + return nil, nil, fmt.Errorf("package has no %s at its root", FileName) + } + if wasmF == nil { + return nil, nil, fmt.Errorf("package has no %s at its root", WasmName) + } + return manifestF, wasmF, nil +} + +func readEntry(f *zip.File, limit int64) ([]byte, error) { + rc, err := f.Open() + if err != nil { + return nil, fmt.Errorf("reading %s: %w", f.Name, err) + } + defer rc.Close() + raw, err := io.ReadAll(io.LimitReader(rc, limit+1)) + if err != nil { + return nil, fmt.Errorf("reading %s: %w", f.Name, err) + } + if int64(len(raw)) > limit { + return nil, fmt.Errorf("%s exceeds the %d byte limit", f.Name, limit) + } + return raw, nil } // WritePackage zips a manifest and module into a .tcade at path.