From 3ffb7e63290fec1f783584421609df3a6f764a9c Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 10:40:01 +0900 Subject: [PATCH 1/8] fix(repo): untrack the retired Go file that reached dev, and guard it mechanically MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit go/internal/cli/config_parity.go was committed by a broad `git add` three times during the #820 campaign. Two of those were caught and reverted (2101d50e5, 58e8718b7); the third rode a86ee03cf into the #892 merge, so 682 lines of the retired Go experiment are now tracked on dev — the one and only tracked file under go/, absent from main, preview, and v2.10.0. Nothing in src/, the build, the typecheck, or the test path reads from go/, so this is dead weight in every clone rather than a functional regression. The commit that introduced it says "untrack ... again" in its own message, which is how it passed review: the intent was right and the index was not. .gitignore alone cannot prevent the repeat, because an already-tracked path stops honoring the ignore rule — so the guard lives in tests/repo-hygiene.test.ts next to the .codexclaw/gitlink invariants, driven red once by re-adding the file. --- .gitignore | 7 + go/internal/cli/config_parity.go | 682 ------------------------------- tests/repo-hygiene.test.ts | 22 + 3 files changed, 29 insertions(+), 682 deletions(-) delete mode 100644 go/internal/cli/config_parity.go diff --git a/.gitignore b/.gitignore index 7f616cf98..d073e9d90 100644 --- a/.gitignore +++ b/.gitignore @@ -39,3 +39,10 @@ devlog/**/security-advisory-draft* # Test-generated artifacts tests/.tmp-*/ .claude/ + +# Retired Go native-runtime experiment. `go/` is not part of the build, the +# typecheck, or the test path, and nothing in `src/` imports it. A single file +# from it (go/internal/cli/config_parity.go) has now been committed by a broad +# `git add` three separate times and reached `dev` once — see +# tests/repo-hygiene.test.ts, which fails if any path here becomes tracked again. +go/ diff --git a/go/internal/cli/config_parity.go b/go/internal/cli/config_parity.go deleted file mode 100644 index dbbf24caf..000000000 --- a/go/internal/cli/config_parity.go +++ /dev/null @@ -1,682 +0,0 @@ -package cli - -import ( - "bytes" - "context" - "encoding/json" - "errors" - "fmt" - "io" - "os" - "path/filepath" - "sort" - "strings" - - "github.com/lidge-jun/opencodex-go/internal/config" -) - -const configUsage = `Usage: - ocx config [show] [--json] [--source] - ocx config get [--json] - ocx config set [--json] - ocx config unset [--json] - ocx config validate [path|-] [--json] - ocx config export - ocx config import --yes [--json]` - -// configDocument is the config as a generic tree, which is what a dot path -// walks. The typed struct cannot represent an arbitrary path. -type configDocument map[string]any - -// readConfigDocument loads the config file as a generic tree plus its -// diagnostics, mirroring readConfigDiagnostics: the config plus where it came -// from and, when the file could not be used, why. -type configDiagnostics struct { - document configDocument - source string - failure string - warnings []string - // order is the key sequence the document should print in. A Go map has - // none, and the oracle prints the order it parsed. - order documentOrder -} - -func readConfigDiagnostics() (configDiagnostics, error) { - path, err := configPath() - if err != nil { - return configDiagnostics{}, err - } - fallback := func(reason string) configDiagnostics { - // The oracle discards an unusable file and hands back defaults, so - // show/get/export never surface its contents. That matters beyond - // tidiness: exporting an unvalidated file would copy whatever - // credentials it holds into a new location. - return configDiagnostics{document: defaultConfigDocument(), source: "fallback", failure: reason, order: defaultDocumentOrder} - } - raw, readErr := os.ReadFile(path) - if readErr != nil { - if os.IsNotExist(readErr) { - return configDiagnostics{document: defaultConfigDocument(), source: "default", order: defaultDocumentOrder}, nil - } - return configDiagnostics{}, readErr - } - // A BOM is stripped the way the oracle does before parsing. - trimmed := strings.TrimPrefix(string(raw), "\ufeff") - var decoded any - if json.Unmarshal([]byte(trimmed), &decoded) != nil { - return fallback("invalid_json"), nil - } - record, isObject := decoded.(map[string]any) - if !isObject { - return fallback("invalid_json"), nil - } - // Degrade before validating: the oracle's schema drops these fields rather - // than rejecting, so a single bad optional value must not send an - // otherwise-good file to fallback. - warnings := degradeInvalidFields(configDocument(record)) - normalized, normalizeErr := normalizeConfigDocument(configDocument(record)) - if normalizeErr != nil { - return fallback(normalizeErr.Error()), nil - } - // The order comes from the SOURCE bytes, not the normalized map, so a - // user's own field sequence survives a round trip through show. - return configDiagnostics{document: normalized, source: "file", warnings: warnings, order: orderOfDocument([]byte(trimmed))}, nil -} - -// readConfigDocument is the common case: the effective config and its origin. -func readConfigDocument() (configDocument, string, error) { - diagnostics, err := readConfigDiagnostics() - if err != nil { - return nil, "", err - } - return diagnostics.document, diagnostics.source, nil -} - -// validateConfigDocument runs the same validation a write would, without -// persisting, so `set` and `import` can refuse an invalid candidate. -func validateConfigDocument(document configDocument) error { - // Structural rules the typed decode cannot express. A missing `providers` - // unmarshals to a nil map and a dangling `defaultProvider` decodes fine, - // so without these an import would write `"providers": null` that the - // oracle rejects outright. - providersValue, hasProviders := document["providers"] - if !hasProviders || providersValue == nil { - return usageError("", "schema_invalid: providers: Invalid input: expected record, received undefined") - } - providers, isObject := providersValue.(map[string]any) - if !isObject { - return usageError("", "schema_invalid: providers: Invalid input: expected record") - } - if selected, present := document["defaultProvider"]; present { - name, isString := selected.(string) - if !isString { - return usageError("", "schema_invalid: defaultProvider: expected string") - } - // No exemption for "openai": the oracle rejects it too when it is - // absent from providers. - if _, known := providers[name]; !known { - return usageError("", "schema_invalid: defaultProvider: defaultProvider must exist in providers") - } - } - encoded, err := json.Marshal(document) - if err != nil { - return err - } - // Decode ONTO the defaults, not onto a zero value. The oracle's schema - // supplies a hostname when the document omits one, so validating a - // zero-valued struct rejected ordinary TypeScript-written configs with - // "hostname: must not be blank" -- a config the TS CLI calls valid. - candidate := config.FreshInstall() - candidate.Providers = nil - candidate.Combos = nil - if err := json.Unmarshal(encoded, &candidate); err != nil { - return usageError("", "%s", err.Error()) - } - return candidate.Validate() -} - -// normalizeConfigDocument validates and returns the document with schema -// defaults MATERIALIZED, the way the oracle's validateConfigCandidate hands -// back a normalized config rather than the raw input. -// -// Without this, a file that legitimately omits `port` validates but then -// `config get port` reports the path as missing, even though the oracle -// resolves it to 10100. -// -// Defaults are layered UNDER the document rather than over it, so a key the -// user actually wrote always wins, and unknown members survive untouched. -func normalizeConfigDocument(document configDocument) (configDocument, error) { - if err := validateConfigDocument(document); err != nil { - return nil, err - } - base := map[string]any(defaultConfigDocument()) - for key, value := range document { - base[key] = value - } - return configDocument(base), nil -} - -// saveConfigDocument writes the VALIDATED GENERIC document, not a typed -// round-trip of it. -// -// Marshalling through config.Config loses any unknown member of a known -// nested object: the root and provider structs carry passthrough fields, but -// something like visionSidecar does not, so `config set port 13000` would -// silently delete visionSidecar.futureNested. Editing one key must never -// discard a setting the user wrote. -// -// The write mirrors config.Save's durability: private temp file in the same -// directory, fsync, atomic rename. -func saveConfigDocument(document configDocument) error { - path, err := configPath() - if err != nil { - return err - } - if err := validateConfigDocument(document); err != nil { - return err - } - encoded, err := json.MarshalIndent(map[string]any(document), "", " ") - if err != nil { - return err - } - encoded = append(encoded, '\n') - - dir := filepath.Dir(path) - if err := os.MkdirAll(dir, 0o700); err != nil { - return fmt.Errorf("create config directory: %w", err) - } - temp, err := os.CreateTemp(dir, ".config-*.tmp") - if err != nil { - return fmt.Errorf("create temporary config: %w", err) - } - tempPath := temp.Name() - committed := false - defer func() { - _ = temp.Close() - if !committed { - _ = os.Remove(tempPath) - } - }() - if err := temp.Chmod(0o600); err != nil { - return fmt.Errorf("protect temporary config: %w", err) - } - if _, err := temp.Write(encoded); err != nil { - return fmt.Errorf("write temporary config: %w", err) - } - if err := temp.Sync(); err != nil { - return fmt.Errorf("sync temporary config: %w", err) - } - if err := temp.Close(); err != nil { - return fmt.Errorf("close temporary config: %w", err) - } - if err := os.Rename(tempPath, path); err != nil { - return fmt.Errorf("replace config: %w", err) - } - committed = true - return nil -} - -// readConfigInput reads a candidate from a file or, for "-", from stdin. -func readConfigInput(source string, stdin io.Reader) (configDocument, error) { - var raw []byte - var err error - if source == "-" { - if stdin == nil { - stdin = os.Stdin - } - raw, err = io.ReadAll(stdin) - } else { - raw, err = os.ReadFile(source) - } - if err != nil { - return nil, err - } - var decoded any - if json.Unmarshal([]byte(strings.TrimPrefix(string(raw), "\ufeff")), &decoded) != nil { - return nil, usageError("", "invalid JSON in %s", source) - } - record, isObject := decoded.(map[string]any) - if !isObject { - return nil, usageError("", "invalid JSON in %s", source) - } - return configDocument(record), nil -} - -// runConfigParity implements the oracle's config surface. The legacy -// fixed-key form stays reachable through runConfig for compatibility. -func runConfigParity(ctx context.Context, args []string, streams IO) error { - rest := append([]string{}, args...) - action := "show" - if len(rest) > 0 { - action = strings.ToLower(rest[0]) - rest = rest[1:] - } - wantsJSON := takeFlag(&rest, "--json") - - switch action { - case "show": - source := takeFlag(&rest, "--source") - if err := rejectArgs(rest, configUsage, false); err != nil { - return err - } - diagnostics, err := readConfigDiagnostics() - if err != nil { - return err - } - redacted, _ := redactConfigValue(map[string]any(diagnostics.document), "").(map[string]any) - if !source { - // show always prints JSON: the oracle passes true for wantsJson. - // It goes through the ordered marshaller so the printed sequence is - // the file's, not Go's map iteration order. - encoded, marshalErr := marshalDocumentInOrder(configDocument(redacted), diagnostics.order) - if marshalErr != nil { - return marshalErr - } - _, writeErr := fmt.Fprintln(streams.Out, string(encoded)) - return writeErr - } - // `error` is present either way, null on success, so a consumer can - // read one shape rather than test for the key. - var failure any - if diagnostics.failure != "" { - failure = diagnostics.failure - } - return printData(streams, map[string]any{ - "config": redacted, - "source": diagnostics.source, - "error": failure, - "warnings": warningList(diagnostics.warnings), - }, true, nil) - - case "get": - if len(rest) == 0 { - return usageError(configUsage, "config path is required") - } - path := rest[0] - rest = rest[1:] - if err := rejectArgs(rest, configUsage, false); err != nil { - return err - } - document, _, err := readConfigDocument() - if err != nil { - return err - } - value, err := getConfigPath(map[string]any(document), path) - if err != nil { - return err - } - segments, err := configPathSegments(path) - if err != nil { - return err - } - value = redactConfigValue(value, segments[len(segments)-1]) - if wantsJSON { - return printData(streams, value, true, nil) - } - text, err := formatConfigValue(value) - if err != nil { - return err - } - _, err = fmt.Fprintln(streams.Out, text) - return err - - case "set", "unset": - if len(rest) == 0 { - return usageError(configUsage, "config path and value are required") - } - path := rest[0] - rest = rest[1:] - var parsed any - if action == "set" { - if len(rest) == 0 { - return usageError(configUsage, "config path and value are required") - } - parsed = parseConfigValue(rest[0]) - rest = rest[1:] - } - if err := rejectArgs(rest, configUsage, false); err != nil { - return err - } - document, _, err := readConfigDocument() - if err != nil { - return err - } - if err := setConfigPath(map[string]any(document), path, parsed, action == "unset"); err != nil { - return err - } - if err := validateConfigDocument(document); err != nil { - return err - } - if err := saveConfigDocument(document); err != nil { - return err - } - var saved any - if action == "set" { - if value, getErr := getConfigPath(map[string]any(document), path); getErr == nil { - segments, _ := configPathSegments(path) - saved = redactConfigValue(value, segments[len(segments)-1]) - } - } - verb := "Set" - if action == "unset" { - verb = "Unset" - } - return printData(streams, map[string]any{"ok": true, "path": path, "value": saved}, - wantsJSON, []string{fmt.Sprintf("%s %s.", verb, path)}) - - case "validate": - source := "" - if len(rest) > 0 { - source = rest[0] - rest = rest[1:] - } - if err := rejectArgs(rest, configUsage, false); err != nil { - return err - } - document := configDocument{} - if source != "" { - loaded, err := readConfigInput(source, streams.In) - if err != nil { - return err - } - document = loaded - } else { - loaded, _, err := readConfigDocument() - if err != nil { - return err - } - document = loaded - } - if err := validateConfigDocument(document); err != nil { - // Invalid config is a reported result, not a crash: the oracle - // prints the reason and exits 1. - if printErr := printData(streams, map[string]any{"ok": false, "error": err.Error()}, - wantsJSON, []string{"Config is invalid: " + err.Error()}); printErr != nil { - return printErr - } - return errSilentFailure - } - reported := source - if reported == "" { - reported, _ = configPath() - } - return printData(streams, map[string]any{"ok": true, "source": reported}, - wantsJSON, []string{"Config is valid."}) - - case "export": - if len(rest) == 0 { - return usageError(configUsage, "export path is required") - } - target := rest[0] - rest = rest[1:] - if err := rejectArgs(rest, configUsage, false); err != nil { - return err - } - document, _, err := readConfigDocument() - if err != nil { - return err - } - // Export is a BACKUP, so it is deliberately not redacted -- a masked - // copy could not be imported back. It is written 0600 for that reason. - encoded, err := json.MarshalIndent(map[string]any(document), "", " ") - if err != nil { - return err - } - encoded = append(encoded, '\n') - if target == "-" { - _, err = streams.Out.Write(encoded) - return err - } - // WriteFile's mode applies only when it CREATES the file, so exporting - // over an existing world-readable path would leave credentials - // readable. Chmod unconditionally. - if err := os.WriteFile(target, encoded, 0o600); err != nil { - return err - } - if err := os.Chmod(target, 0o600); err != nil { - return fmt.Errorf("protect exported config: %w", err) - } - _, err = fmt.Fprintf(streams.Out, "Exported config to %s.\n", target) - return err - - case "import": - if len(rest) == 0 { - return usageError(configUsage, "import path is required") - } - source := rest[0] - rest = rest[1:] - yes := takeFlag(&rest, "--yes") - if !yes { - return usageError(configUsage, "import requires --yes") - } - if err := rejectArgs(rest, configUsage, false); err != nil { - return err - } - document, err := readConfigInput(source, streams.In) - if err != nil { - return err - } - if err := validateConfigDocument(document); err != nil { - return err - } - if err := saveConfigDocument(document); err != nil { - return err - } - return printData(streams, map[string]any{"ok": true, "source": source}, wantsJSON, - []string{fmt.Sprintf("Imported config from %s. Restart or run ocx sync if needed.", source)}) - } - return usageError(configUsage, "unknown config command %s", action) -} - -// errSilentFailure marks a failure the command has ALREADY reported, so Run -// exits non-zero without printing a second "Error:" line over the top of it. -var errSilentFailure = errors.New("reported failure") - -// defaultConfigDocument is the generic form of the built-in default config. -// -// The oracle answers an absent or unusable config with getDefaultConfig() -// rather than an empty object, so `validate` succeeds on a fresh home and -// `get providers.openai.adapter` resolves before the user has written anything. -func defaultConfigDocument() configDocument { - // Built from FreshInstall, then reconciled with the oracle's - // getDefaultConfig() SHAPE. - // - // The two are not the same document. Go's struct marshals hostname, debug - // and log that the oracle omits, and the oracle carries websockets:false - // that Go's zero value drops. Serving or persisting the Go shape would - // write a config the TypeScript CLI did not produce, so the extras are - // removed and the missing key restored. - defaults := config.FreshInstall() - encoded, err := json.Marshal(defaults) - if err != nil { - return configDocument{} - } - var document map[string]any - if json.Unmarshal(encoded, &document) != nil { - return configDocument{} - } - for _, goOnly := range []string{"hostname", "debug", "log", "streamMode"} { - delete(document, goOnly) - } - if _, present := document["websockets"]; !present { - document["websockets"] = false - } - return configDocument(document) -} - -// degradableFields are the schema entries the oracle declares with -// `.catch(undefined)`: an invalid value is DROPPED with a warning rather than -// rejecting the whole file, so one hand-edited typo cannot hide every provider -// and account the user has configured. -var degradableFields = map[string]string{ - "injectionModel": "a string", - "injectionEffort": "a string", - "streamMode": "a string", - "syncCodexSubagentDefaults": "a boolean", -} - -// degradeInvalidFields removes malformed optional fields and reports what it -// dropped, in the oracle's wording. -func degradeInvalidFields(document configDocument) []string { - warnings := []string{} - for _, field := range []string{"injectionModel", "injectionEffort", "streamMode", "syncCodexSubagentDefaults"} { - value, present := document[field] - if !present || value == nil { - continue - } - expected := degradableFields[field] - valid := false - switch typed := value.(type) { - case string: - valid = expected == "a string" - if field == "streamMode" && valid { - valid = typed == "auto" || typed == "legacy-tee" || typed == "eager-relay" - } - case bool: - valid = expected == "a boolean" - } - if !valid { - delete(document, field) - warnings = append(warnings, field+" ignored: expected "+expected) - } - } - return warnings -} - -// warningList renders warnings as a JSON array, empty rather than null when -// there are none. -func warningList(warnings []string) []any { - out := make([]any, 0, len(warnings)) - for _, warning := range warnings { - out = append(out, warning) - } - return out -} - -// documentOrder is the ordered form of a whole config document. -// -// A Go map has no key order and JSON.stringify preserves the one it parsed, so -// `config show` printed alphabetically where the oracle prints file order. The -// order is tracked beside the document rather than inside it, because every -// dot-path walk in this file relies on plain map lookup. -type documentOrder struct { - value orderedValue - ok bool -} - -// orderOfDocument records the key sequence, at every depth, from the source -// bytes. -func orderOfDocument(raw []byte) documentOrder { - value, err := decodeOrdered(raw) - if err != nil || value.kind != 'o' { - return documentOrder{} - } - return documentOrder{value: value, ok: true} -} - -// defaultDocumentOrder is the oracle's getDefaultConfig() literal order, used -// when there is no file to read an order from. -var defaultDocumentOrder = orderOfDocument([]byte(`{ - "port": 0, - "openaiProviderTierVersion": 0, - "providers": {"openai": {"adapter": "", "baseUrl": "", "authMode": "", "codexAccountMode": ""}}, - "defaultProvider": "", - "subagentModels": [], - "multiAgentGuidanceEnabled": false, - "websockets": false, - "codexAutoStart": false, - "codexShimAutoRestore": false -}`)) - -// marshalDocumentInOrder renders the document following the recorded key order -// at each level, appending any key the order does not mention in sorted order -// so the output stays deterministic. -func marshalDocumentInOrder(document configDocument, order documentOrder) ([]byte, error) { - var reference *orderedValue - if order.ok { - reference = &order.value - } - compact, err := orderedJSONBytes(map[string]any(document), reference) - if err != nil { - return nil, err - } - var indented bytes.Buffer - if err := json.Indent(&indented, compact, "", " "); err != nil { - return nil, err - } - return indented.Bytes(), nil -} - -// orderedJSONBytes serializes value, taking key order from reference when the -// two line up and falling back to sorted keys when they do not. -func orderedJSONBytes(value any, reference *orderedValue) ([]byte, error) { - record, isObject := value.(map[string]any) - if !isObject { - if items, isArray := value.([]any); isArray { - out := []byte{'['} - for index, item := range items { - if index > 0 { - out = append(out, ',') - } - var childReference *orderedValue - if reference != nil && reference.kind == 'a' && index < len(reference.values) { - childReference = &reference.values[index] - } - encoded, err := orderedJSONBytes(item, childReference) - if err != nil { - return nil, err - } - out = append(out, encoded...) - } - return append(out, ']'), nil - } - return json.Marshal(jsSafe(value)) - } - - keys := make([]string, 0, len(record)) - seen := make(map[string]struct{}, len(record)) - if reference != nil && reference.kind == 'o' { - for _, key := range reference.keys { - if _, present := record[key]; present { - keys = append(keys, key) - seen[key] = struct{}{} - } - } - } - remaining := make([]string, 0, len(record)) - for key := range record { - if _, already := seen[key]; !already { - remaining = append(remaining, key) - } - } - sort.Strings(remaining) - keys = append(keys, remaining...) - - out := []byte{'{'} - for index, key := range keys { - if index > 0 { - out = append(out, ',') - } - encodedKey, err := json.Marshal(key) - if err != nil { - return nil, err - } - var childReference *orderedValue - if reference != nil && reference.kind == 'o' { - for position, candidate := range reference.keys { - if candidate == key { - childReference = &reference.values[position] - break - } - } - } - encodedValue, err := orderedJSONBytes(record[key], childReference) - if err != nil { - return nil, err - } - out = append(out, encodedKey...) - out = append(out, ':') - out = append(out, encodedValue...) - } - return append(out, '}'), nil -} diff --git a/tests/repo-hygiene.test.ts b/tests/repo-hygiene.test.ts index ad5f87ec5..7f699fc9c 100644 --- a/tests/repo-hygiene.test.ts +++ b/tests/repo-hygiene.test.ts @@ -19,6 +19,16 @@ const FORBIDDEN_TRACKED_DIRS = [".codexclaw", ".omo", ".claude", "node_modules", const FORBIDDEN_TRACKED_FILENAMES = [".DS_Store", "Thumbs.db"]; +/** + * The retired Go native-runtime experiment. Nothing in `src/`, the build, the + * typecheck, or the test path reads from `go/`, so a tracked file there is always + * an accident — and this specific one is a repeat offender: `git add -A` pulled + * `go/internal/cli/config_parity.go` back into the index three times during the + * #820 campaign, and the third one rode a merge into `dev`. `.gitignore` cannot + * catch that on its own, because an already-tracked path ignores the rule. + */ +const RETIRED_TRACKED_DIRS = ["go"]; + function trackedFiles(): string[] { const result = Bun.spawnSync(["git", "ls-files"], { cwd: repoRoot }); if (result.exitCode !== 0) { @@ -64,12 +74,24 @@ describe("repository hygiene", () => { expect(offenders).toEqual([]); }); + test("the retired Go runtime stays untracked", () => { + const offenders = trackedFiles().filter((path) => + RETIRED_TRACKED_DIRS.some((dir) => path === dir || path.startsWith(`${dir}/`)), + ); + + expect(offenders).toEqual([]); + }); + test("gitignore still declares the agent-state directories", async () => { const ignore = await Bun.file(new URL("../.gitignore", import.meta.url)).text(); for (const dir of FORBIDDEN_TRACKED_DIRS) { expect(ignore).toContain(`${dir}/`); } + + for (const dir of RETIRED_TRACKED_DIRS) { + expect(ignore).toContain(`${dir}/`); + } }); }); From 503786a6ac2bc073b7b74793873e3abe4cb8e851 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 10:48:43 +0900 Subject: [PATCH 2/8] fix(consent): key the star guard on the authenticating credential, not headers The agent-consent refusal on POST /api/github/star asked whether the request carried an Origin plus the GUI-origin and CSRF headers, on the stated premise that requireManagementAuth had already matched them against a minted session. It had not. The gate accepts a raw admin token and returns BEFORE it consults the session table, so those headers were never validated for a token-authorized call, and the admin token is readable by anything running as the user, which is precisely the caller this guard exists to refuse. Three headers with arbitrary values were enough to star the repository with the user's identity. managementPrincipal() now resolves which credential passed the gate, from the same session table and the same CSRF comparison the gate uses, and the server passes it into the management dispatcher. The route asks for a gui-session principal: a session this process minted for a browser, which is only accepted for a mutation after origin and per-session CSRF both match. An unresolved principal (direct dispatch in tests, any future internal caller) is untrusted. Behavior for real users is unchanged: dashboard clicks still star, hand-typed runs still star, and the non-loopback operator dashboard on a raw admin token keeps the documented fail-closed edge. Both regressions were driven red against the old header check. --- src/server/index.ts | 8 +++- src/server/management-api.ts | 11 +++++- src/server/management-auth.ts | 32 +++++++++++++++ src/server/management/context.ts | 10 +++++ src/server/management/sidebar-routes.ts | 29 +++++++------- tests/sidebar-routes.test.ts | 52 +++++++++++++++++++++++-- tests/startup-prompt.test.ts | 7 +++- 7 files changed, 127 insertions(+), 22 deletions(-) diff --git a/src/server/index.ts b/src/server/index.ts index 0b1e323d8..5220a4d17 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -157,7 +157,7 @@ import { handleImages } from "./images"; import { handleLive, logLiveSidebandFrame, parseLiveSidebandTarget, resolveLiveSidebandUpgrade } from "./live"; import { handleSearch } from "./search"; import { fetchAllModels, handleManagementAPI, VERSION } from "./management-api"; -import { initializeManagementAuthState, issueGuiSession, requireManagementAuth } from "./management-auth"; +import { initializeManagementAuthState, issueGuiSession, managementPrincipal, requireManagementAuth } from "./management-auth"; const MAX_WS_FRAME_BYTES = 50 * 1024 * 1024; const WEBSOCKET_IDLE_TIMEOUT_SECONDS = 0; @@ -448,7 +448,11 @@ export function startServer(port?: number) { if (url.pathname.startsWith("/api/")) { const apiAuthError = requireManagementAuth(req, managementAuth, config); if (apiAuthError) return withManagementCors(apiAuthError, req, config); - const mgmtResponse = await handleManagementAPI(req, url, config); + // Which credential passed the gate, resolved from the same session table the + // gate used. Consent-bearing routes need this: request headers are forgeable + // by anything holding the admin token, the credential is not. + const principal = managementPrincipal(req, managementAuth, config) ?? undefined; + const mgmtResponse = await handleManagementAPI(req, url, config, {}, principal); if (mgmtResponse) return withManagementCors(mgmtResponse, req, config); return withManagementCors(formatErrorResponse(404, "not_found", `Unknown endpoint: ${req.method} ${url.pathname}`), req, config); } diff --git a/src/server/management-api.ts b/src/server/management-api.ts index ad5ebc896..af4b6bf9a 100644 --- a/src/server/management-api.ts +++ b/src/server/management-api.ts @@ -68,6 +68,7 @@ import { handleSystemRoutes } from "./management/system-routes"; import { handleSidebarRoutes } from "./management/sidebar-routes"; import { handleIntegrationRoutes } from "./management/integration-routes"; import type { ManagementContext } from "./management/context"; +import type { ManagementPrincipal } from "./management-auth"; export type { ManagementApiDeps } from "./management/context"; import { fetchAllModels } from "./management/shared"; import { CatalogGatherBusyError } from "../codex/catalog/provider-fetch"; @@ -82,7 +83,13 @@ export const VERSION = (() => { } })(); -export async function handleManagementAPI(req: Request, url: URL, config: OcxConfig, deps: ManagementApiDeps = {}): Promise { +export async function handleManagementAPI( + req: Request, + url: URL, + config: OcxConfig, + deps: ManagementApiDeps = {}, + principal?: ManagementPrincipal, +): Promise { if (!isAllowedManagementOrigin(req, config)) { return jsonResponse({ error: "cross-origin request blocked" }, 403, req, config); } @@ -125,7 +132,7 @@ export async function handleManagementAPI(req: Request, url: URL, config: OcxCon } } catch { /* best-effort */ } } - const ctx: ManagementContext = { req, url, config, deps, refreshCodexCatalogBestEffort, syncClaudeAgentDefsBestEffort }; + const ctx: ManagementContext = { req, url, config, deps, principal, refreshCodexCatalogBestEffort, syncClaudeAgentDefsBestEffort }; let routed: Response | null; try { routed = (await handleConfigRoutes(ctx)) diff --git a/src/server/management-auth.ts b/src/server/management-auth.ts index 813179636..1f1d72b53 100644 --- a/src/server/management-auth.ts +++ b/src/server/management-auth.ts @@ -237,6 +237,38 @@ export function issueGuiSession( return { token, ...session }; } +/** + * Which credential actually authorized a management request. + * + * `admin-token` is the raw token from disk/env: anything running as the user can + * read it, including a coding agent. `gui-session` is a session token this process + * minted for a browser, and it only authorizes a mutation after the origin and the + * per-session CSRF token match. Consent-bearing routes must key off this value + * rather than off request headers, which the token holder can forge freely. + */ +export type ManagementPrincipal = "admin-token" | "gui-session"; + +/** + * The principal for a request that already passed `requireManagementAuth`. Kept as a + * separate resolution (rather than a changed return type) so every existing caller + * keeps its `Response | null` contract; the value is derived from the same session + * table and the same CSRF comparison the gate uses, so the two cannot disagree. + */ +export function managementPrincipal( + req: Request, + state: ManagementAuthState, + config?: OcxConfig, +): ManagementPrincipal | null { + if (!state.available) return null; + const actual = req.headers.get("x-opencodex-api-key")?.trim() + || req.headers.get("authorization")?.replace(/^Bearer\s+/i, "").trim(); + if (!actual) return null; + if (equalSecret(actual, state.token)) return "admin-token"; + if (!config) return null; + removeExpiredSessions(state); + return state.sessions.has(actual) ? "gui-session" : null; +} + export function requireManagementAuth( req: Request, state: ManagementAuthState, diff --git a/src/server/management/context.ts b/src/server/management/context.ts index 61d37e7a7..aee70b9ee 100644 --- a/src/server/management/context.ts +++ b/src/server/management/context.ts @@ -1,5 +1,6 @@ import type { OcxConfig } from "../../types"; import type { StartupInstallAction } from "../startup-action-control"; +import type { ManagementPrincipal } from "../management-auth"; export interface ManagementApiDeps { toggleCodexMultiAgentV2?: (enabled: boolean) => void; @@ -26,6 +27,15 @@ export interface ManagementContext { url: URL; config: OcxConfig; deps: ManagementApiDeps; + /** + * Which credential authorized this request, resolved by the auth gate before + * dispatch. Routes that spend the USER's identity (not just the proxy's) must + * branch on this instead of on request headers: the admin token is readable by + * anything running as the user, so a token holder can forge any header a route + * might otherwise treat as browser evidence. Undefined only in direct-dispatch + * tests, which are treated as the untrusted `admin-token` case. + */ + principal?: ManagementPrincipal; refreshCodexCatalogBestEffort: () => Promise; syncClaudeAgentDefsBestEffort: () => Promise; } diff --git a/src/server/management/sidebar-routes.ts b/src/server/management/sidebar-routes.ts index fa9366918..2ebc11d5f 100644 --- a/src/server/management/sidebar-routes.ts +++ b/src/server/management/sidebar-routes.ts @@ -16,25 +16,28 @@ * * The dashboard button must keep working even when the proxy itself was started by * an agent, which is the common case — the person is at the browser, not at the - * spawning shell. A GUI click is therefore distinguished by its browser session - * evidence (a same-origin `Origin` plus the minted CSRF header, both already - * verified by the management auth gate) rather than by the proxy's own env. + * spawning shell. A GUI click is therefore distinguished by the CREDENTIAL that + * authorized the request — a GUI session this process minted for a browser, which + * the auth gate only accepts after matching origin and the per-session CSRF token — + * rather than by the proxy's own env or by request headers. */ import { jsonResponse } from "../auth-cors"; import { agentDrivenMarkers, isAgentDriven } from "../../cli/agent-driven"; import type { ManagementContext } from "./context"; /** - * True when this request carries the browser-session evidence a dashboard click - * always has: an `Origin` (only a browser sends one) plus the per-session CSRF - * token, which `requireManagementAuth` has already matched against the minted - * session before dispatch. A shell/HTTP caller holding only the admin token has - * neither, which is exactly the case the agent guard is aimed at. + * True only when a minted GUI session authorized this request. + * + * The previous version of this check looked for an `Origin` plus the CSRF headers + * and reasoned that the auth gate had already validated them. It had not: the gate + * accepts a raw admin token BEFORE it ever consults the session table, so a caller + * holding that token (any process running as the user, a coding agent included) + * could add three nonempty headers of its choosing and satisfy this check without + * a browser ever being involved. The credential itself is the only part of the + * request an agent cannot fabricate, so that is what this now reads. */ -function hasBrowserSessionEvidence(req: Request): boolean { - return !!req.headers.get("Origin")?.trim() - && !!req.headers.get("x-opencodex-csrf-token")?.trim() - && !!req.headers.get("x-opencodex-gui-origin")?.trim(); +function hasBrowserSessionEvidence(ctx: ManagementContext): boolean { + return ctx.principal === "gui-session"; } // Known edge, deliberately fail-closed: a non-loopback operator dashboard signs in @@ -58,7 +61,7 @@ export async function handleSidebarRoutes(ctx: ManagementContext): Promise = {}, + principal?: "admin-token" | "gui-session", ): Promise<{ status: number; body: unknown; raw: string; routed: boolean }> { // `isAllowedManagementOrigin` derives the expected origin from the Host header and // rejects the request outright when it is missing, so Host is required here. Omitting // Origin models the GUI's own same-origin fetch. const url = new URL(`http://127.0.0.1:10100${pathname}`); const req = new Request(url, { method, headers: { host: "127.0.0.1:10100", ...headers } }); - const res = await handleManagementAPI(req, url, config); + const res = await handleManagementAPI(req, url, config, {}, principal); if (!res) return { status: 404, body: null, raw: "", routed: false }; const raw = await res.text(); return { status: res.status, body: raw ? JSON.parse(raw) : null, raw, routed: true }; @@ -183,19 +184,62 @@ describe("route surface", () => { async runGh(args) { calls.push(args); return { status: 0 }; }, }, async () => { invalidateStarStatusCache(); - // Browser-session evidence: same-origin Origin plus the minted CSRF/GUI-origin - // headers the management auth gate has already verified before dispatch. + // Browser-session evidence is the CREDENTIAL, not the headers: the auth gate + // resolved a minted GUI session, which it only issues to a browser and only + // accepts for a mutation after matching origin and the per-session CSRF token. const { status, body } = await call("POST", "/api/github/star", { origin: "http://127.0.0.1:10100", "x-opencodex-gui-origin": "http://127.0.0.1:10100", "x-opencodex-csrf-token": "csrf-token", - }); + }, "gui-session"); expect(status).toBe(200); expect((body as Record).ok).toBe(true); })); expect(calls.some(args => args.includes("PUT"))).toBe(true); }); + test("forged dashboard headers on an admin-token call cannot star", async () => { + // The consent guard used to read the request's Origin/CSRF/GUI-origin headers and + // trust them as proof of a browser click. The auth gate accepts a raw admin token + // BEFORE it consults the session table, so an agent that can read that token — any + // process running as the user — could send exactly these headers with arbitrary + // values and star the repository with the user's identity. + const calls: string[][] = []; + await withEnv({ ...NO_AGENT_ENV, CODEX_THREAD_ID: "019fbc94" }, () => withStarDeps({ + nowMs: () => 0, + async runGh(args) { calls.push(args); return { status: 0 }; }, + }, async () => { + invalidateStarStatusCache(); + const { status, body } = await call("POST", "/api/github/star", { + origin: "http://127.0.0.1:10100", + "x-opencodex-gui-origin": "http://127.0.0.1:10100", + "x-opencodex-csrf-token": "forged-by-the-token-holder", + }, "admin-token"); + expect(status).toBe(403); + expect((body as Record).code).toBe("agent_consent_required"); + })); + expect(calls).toEqual([]); + }); + + test("a direct dispatch with no resolved principal is treated as untrusted", async () => { + // Defense in depth for callers that bypass the HTTP gate (route-level tests, future + // internal dispatchers): an unknown principal must never satisfy the consent check. + const calls: string[][] = []; + await withEnv({ ...NO_AGENT_ENV, CODEX_THREAD_ID: "019fbc94" }, () => withStarDeps({ + nowMs: () => 0, + async runGh(args) { calls.push(args); return { status: 0 }; }, + }, async () => { + invalidateStarStatusCache(); + const { status } = await call("POST", "/api/github/star", { + origin: "http://127.0.0.1:10100", + "x-opencodex-gui-origin": "http://127.0.0.1:10100", + "x-opencodex-csrf-token": "csrf-token", + }); + expect(status).toBe(403); + })); + expect(calls).toEqual([]); + }); + test("a hand-typed run is not blocked by the agent guard", async () => { const calls: string[][] = []; await withEnv(NO_AGENT_ENV, () => withStarDeps({ diff --git a/tests/startup-prompt.test.ts b/tests/startup-prompt.test.ts index 52208e502..529fb8ef4 100644 --- a/tests/startup-prompt.test.ts +++ b/tests/startup-prompt.test.ts @@ -120,7 +120,12 @@ describe("startup star prompt", () => { // A dashboard click must still work when an agent started the proxy, so the // refusal is conditioned on the absence of browser-session evidence. expect(routes).toContain("hasBrowserSessionEvidence"); - expect(routes).toMatch(/isAgentDriven\(\)\s*&&\s*!hasBrowserSessionEvidence\(req\)/); + expect(routes).toMatch(/isAgentDriven\(\)\s*&&\s*!hasBrowserSessionEvidence\(ctx\)/); + // And that evidence must be the authenticating CREDENTIAL, never a request + // header: the admin token is readable by anything running as the user, so a + // header-shaped check is forgeable by the exact caller this guard refuses. + expect(routes).toMatch(/principal === "gui-session"/); + expect(routes).not.toMatch(/hasBrowserSessionEvidence[\s\S]*?headers\.get\("x-opencodex-csrf-token"\)/); }); test("the consent rule is written down where agents and users read it", async () => { From 5d8acf91ce308731d09702cd7121fc3e65cd59c9 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 10:50:52 +0900 Subject: [PATCH 3/8] fix(relay-eager): wake a parked read by cancelling the reader, not by racing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The eager producer raced every read against one never-settled abort promise. Each completed read leaves a reaction attached to that pending promise, so a long stream retained one callback per chunk until abort — the exact retention class relay.ts documents avoiding at its own drain ("Deliberately NOT a shared Promise.race companion"), reintroduced in the relay this campaign added. Abort now cancels the reader instead, which settles the parked read on a silent upstream the same way relay.ts's stopDrain does, and the loop checks the signal once per iteration. The 31 eager-relay tests — cancel-drain expiry, shutdown while paused, synthetic tails, teardown, and rewrite framing — stay green, which is what proves the wake-up path is unchanged. --- src/server/relay-eager.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/server/relay-eager.ts b/src/server/relay-eager.ts index e208c0920..f580e9624 100644 --- a/src/server/relay-eager.ts +++ b/src/server/relay-eager.ts @@ -194,17 +194,19 @@ export function relaySseEagerBounded( let syntheticKind: "incomplete" | "failed" | null = null; // reader.read() is not intrinsically tied to the upstream AbortController // (a fetch body usually rejects on abort, but that coupling is the fetch - // implementation's, not the stream's). Race every read against the abort - // signal so cancel-drain expiry and shutdown teardown ALWAYS break the - // loop even on a silent upstream. - const aborted: Promise<"aborted"> = new Promise(resolve => { - if (upstream.signal.aborted) resolve("aborted"); - else upstream.signal.addEventListener("abort", () => resolve("aborted"), { once: true }); - }); + // implementation's, not the stream's), so abort must break a parked read on + // a silent upstream. Cancelling the reader does that: the pending read + // settles and the loop observes the abort. This is deliberately NOT a + // shared `Promise.race([reader.read(), aborted])` companion — racing every + // read against one never-settled promise retains a reaction per chunk, and + // that is the exact retention class relay.ts avoids at its own drain. + const wakeParkedRead = () => { reader.cancel(upstream.signal.reason).catch(() => {}); }; + if (upstream.signal.aborted) wakeParkedRead(); + else upstream.signal.addEventListener("abort", wakeParkedRead, { once: true }); try { for (;;) { - const result = await Promise.race([reader.read(), aborted]); - if (result === "aborted") break; + const result = await reader.read(); + if (upstream.signal.aborted) break; const { done: upstreamDone, value } = result; if (upstreamDone) { hooks.finishInspection(); From 99747ca8286f69a09458c73476db5256f5b2d097 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 10:51:56 +0900 Subject: [PATCH 4/8] test(bounded-body): lock the explicit maxBytes budget and fragmented reassembly MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The maxBytes option was mutation-surviving: deleting it and always using the 64 KiB default left the whole suite green, because the only oversize test used a 33 MiB body that exceeds both ceilings. Nothing proved that the one caller the option exists for — the non-streaming upstream JSON read, at a 32 MiB ceiling — can actually accept a response larger than an error body. Four cases now pin it: a body between the default and the custom cap succeeds, the exact cap succeeds, one byte past it fails closed with the prefix discarded, and a 20k-chunk fragmented body under the cap reassembles byte-exactly (the observable half of the geometric single-buffer accumulation). Driven red by re-ignoring the option. --- tests/bounded-body.test.ts | 59 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/tests/bounded-body.test.ts b/tests/bounded-body.test.ts index 93ca2d5e7..18989bd1b 100644 --- a/tests/bounded-body.test.ts +++ b/tests/bounded-body.test.ts @@ -105,6 +105,65 @@ describe("readBoundedResponseBody", () => { expect(cancelled).toBe(true); }); + /** + * The `maxBytes` option exists so one caller — the non-streaming upstream JSON + * read in responses/core.ts — can accept a whole completion (32 MiB ceiling) + * while the other eight callers keep the 64 KiB error-body default. Without + * these three tests the option was mutation-surviving: ignoring `maxBytes` + * entirely left the suite green, because the only oversize test used a body + * that exceeds BOTH ceilings. + */ + describe("an explicit maxBytes budget", () => { + const CUSTOM_CAP = BOUNDED_BODY_MAX_BYTES * 4; + + test("accepts a body larger than the default but within the custom cap", async () => { + const size = BOUNDED_BODY_MAX_BYTES * 2; + const response = responseFromChunks(new Uint8Array(size).fill(0x61)); + + const result = await readBoundedResponseBody(response, { maxBytes: CUSTOM_CAP }); + + expect(result.text.length).toBe(size); + expect(result.oversized).toBe(false); + expect(result.truncated).toBe(false); + expect(result.displaySafe).toBe(true); + }); + + test("accepts exactly the custom cap", async () => { + const response = responseFromChunks(new Uint8Array(CUSTOM_CAP).fill(0x61)); + + const result = await readBoundedResponseBody(response, { maxBytes: CUSTOM_CAP }); + + expect(result.text.length).toBe(CUSTOM_CAP); + expect(result.oversized).toBe(false); + }); + + test("rejects one byte past the custom cap and discards the prefix", async () => { + const response = responseFromChunks(new Uint8Array(CUSTOM_CAP + 1).fill(0x61)); + + const result = await readBoundedResponseBody(response, { maxBytes: CUSTOM_CAP }); + + expect(result.text).toBe(""); + expect(result.oversized).toBe(true); + expect(result.displaySafe).toBe(false); + }); + + test("a highly fragmented body under the cap is reassembled exactly", async () => { + // Guards the geometric single-buffer accumulation: the previous per-chunk + // array retained one object per transport chunk, which a peer can inflate + // far beyond the payload ceiling. Correctness here is the observable part — + // 20k one-byte chunks must still decode to exactly their content. + const chunkCount = 20_000; + const chunks = Array.from({ length: chunkCount }, () => new Uint8Array([0x61])); + const response = responseFromChunks(...chunks); + + const result = await readBoundedResponseBody(response, { maxBytes: CUSTOM_CAP }); + + expect(result.text.length).toBe(chunkCount); + expect(result.text).toBe("a".repeat(chunkCount)); + expect(result.oversized).toBe(false); + }); + }); + test("parent abort rejects with the exact reason object", async () => { const controller = new AbortController(); const reason = { code: "parent-stopped" }; From f2eb047afb250f06dc7c7a3a24f3d2650879e71e Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 10:54:13 +0900 Subject: [PATCH 5/8] test: close three coverage gaps the post-merge audit found mutation-surviving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All three guard real behavior that no test actually pinned: - antigravity canonicalization: removing MAX_CANONICAL_DEPTH left all 34 tests green. Byte and key budgets do not bound recursion — a deeply nested argument is tiny on the wire — so without the cap a replay observation throws RangeError instead of skipping. Depth 120 canonicalizes, 200 and 50k refuse with null. - scheduler settle predicate: the end-to-end unknown-SCM test sets taskInstalled and registrationHealthy true, so its final clause is already false and deleting the nativeServiceAbsent guard left it green. schedulerVerificationMaySettle is now exercised directly against a transient-looking tail, one unproven flag at a time, and goes red when that guard is removed. - ephemeral ACL memo release: the test inherited USERNAME from an earlier block's `??=`, which never restores it, so running it alone or in another order failed before reaching the memo behavior. It sets and restores its own environment now. --- tests/google-antigravity-replay.test.ts | 19 +++++++++++++ tests/windows-elevation-spawn.test.ts | 38 +++++++++++++++++++++++++ tests/windows-secret-acl.test.ts | 8 ++++++ 3 files changed, 65 insertions(+) diff --git a/tests/google-antigravity-replay.test.ts b/tests/google-antigravity-replay.test.ts index 00a586b1b..64962ee3d 100644 --- a/tests/google-antigravity-replay.test.ts +++ b/tests/google-antigravity-replay.test.ts @@ -413,6 +413,25 @@ describe("antigravity replay fixed-size key identities", () => { expect(antigravityCanonicalJsonBoundedForTests({ a: [1, "x"] }, 1024)).toBe('{"a":[1,"x"]}'); }); + test("pathological nesting is refused by the depth cap, not by a stack overflow", () => { + // The byte and key budgets do not bound RECURSION: a deeply nested argument + // shape is tiny on the wire. Without the depth cap this walk exhausts the + // stack and throws RangeError out of a replay observation, which is a crash + // path rather than a skipped replay. Removing MAX_CANONICAL_DEPTH left every + // other antigravity test green, so this is the only case that pins it. + const nest = (levels: number): unknown => { + let value: unknown = 1; + for (let i = 0; i < levels; i++) value = { n: value }; + return value; + }; + + // Comfortably inside the cap: canonicalizes normally. + expect(antigravityCanonicalJsonBoundedForTests(nest(120), 1024 * 1024)).toContain('{"n":'); + // Past the cap: a null refusal, never a thrown RangeError. + expect(antigravityCanonicalJsonBoundedForTests(nest(200), 1024 * 1024)).toBeNull(); + expect(antigravityCanonicalJsonBoundedForTests(nest(50_000), 8 * 1024 * 1024)).toBeNull(); + }); + test("overflow aborts the walk near the cap, proven by scan instrumentation", () => { resetCanonicalScanUnitsForTests(); const hugeString = "y".repeat(10 * 1024 * 1024); diff --git a/tests/windows-elevation-spawn.test.ts b/tests/windows-elevation-spawn.test.ts index 40b7243ae..0c7bab727 100644 --- a/tests/windows-elevation-spawn.test.ts +++ b/tests/windows-elevation-spawn.test.ts @@ -22,6 +22,7 @@ import { import { evaluateSchedulerInstallRestartReconciliation, finalizeWindowsSchedulerServiceRegistration, + schedulerVerificationMaySettle, setFinalizeWindowsSchedulerHooksForTests, } from "../src/service"; import type { WindowsSchedulerInstallVerification } from "../src/service"; @@ -987,6 +988,43 @@ describe("finalizeWindowsSchedulerServiceRegistration", () => { expect(parentRollbackLaunches).toBe(0); }); + test("the settle predicate refuses every unproven state on its own terms", () => { + // The end-to-end unknown-SCM test above cannot isolate this: its fixture has + // taskInstalled and registrationHealthy both true, so the FINAL clause is + // already false and deleting an earlier guard leaves it green. Exercising the + // predicate directly with a transient-looking tail (invisible task) is what + // proves each guard carries its own weight. + const transientTail: WindowsSchedulerInstallVerification = { + taskInstalled: false, + registrationHealthy: false, + registrationInvalid: false, + assetsHealthy: true, + nativeServiceAbsent: true, + nativeStatusUnknown: false, + conflict: false, + ok: false, + detail: "Task Scheduler task is not installed.", + }; + + // Baseline: a scheduler view that has genuinely not caught up may settle. + expect(schedulerVerificationMaySettle(transientTail)).toBe(true); + + // Each of these is unproven or permanent, and must refuse even though the + // transient tail below it still looks retryable. + expect(schedulerVerificationMaySettle({ ...transientTail, ok: true })).toBe(false); + expect(schedulerVerificationMaySettle({ ...transientTail, conflict: true })).toBe(false); + expect(schedulerVerificationMaySettle({ ...transientTail, assetsHealthy: false })).toBe(false); + expect(schedulerVerificationMaySettle({ ...transientTail, nativeServiceAbsent: false })).toBe(false); + expect(schedulerVerificationMaySettle({ ...transientTail, registrationInvalid: true })).toBe(false); + + // And a fully healthy-but-not-ok view has nothing left to wait for. + expect(schedulerVerificationMaySettle({ + ...transientTail, + taskInstalled: true, + registrationHealthy: true, + })).toBe(false); + }); + test("ownership lost during a settle delay stops without rollback or state write", async () => { mockParentRollbackSpawn(); let owned = true; diff --git a/tests/windows-secret-acl.test.ts b/tests/windows-secret-acl.test.ts index cea08c21a..ce6e4edb1 100644 --- a/tests/windows-secret-acl.test.ts +++ b/tests/windows-secret-acl.test.ts @@ -626,6 +626,12 @@ describe("ephemeral ACL memo release (#840 refinement)", () => { test("ephemeral release clears temp-keyed timeout memos in BOTH namespaces", () => { setPlatformForTests("win32"); setIcaclsRunnerForTests(() => timeout); + // Own the environment this test needs. It used to inherit USERNAME from an + // earlier describe's `??=`, which never restores it: run this file's blocks in + // another order, or this test alone, and the harden fails before it ever + // reaches the memo behavior under test. + const previousUsername = process.env.USERNAME; + process.env.USERNAME = "ocx-test-user"; const tempA = join(testDir, "dest.ocx.1.1.tmp"); const tempB = join(testDir, "dest.ocx.1.2.tmp"); writeFileSync(tempA, "a", "utf-8"); @@ -642,6 +648,8 @@ describe("ephemeral ACL memo release (#840 refinement)", () => { } finally { setIcaclsRunnerForTests(null); setPlatformForTests(null); + if (previousUsername === undefined) delete process.env.USERNAME; + else process.env.USERNAME = previousUsername; } }); From ef851ae983ab2ca3401fb9eaf32bcf018bf86cca Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 11:08:58 +0900 Subject: [PATCH 6/8] fix(consent,relay-eager): require a dashboard session, and inspect before abort MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two findings from the second audit round. The star mutation required a GUI session only when isAgentDriven() was true, and that function reads the SERVER's environment rather than the caller's. A proxy running as a service — no agent markers, the normal remote setup — therefore accepted a raw-admin-token star from anyone who could read the token, which is every agent on the machine. Caller provenance is not knowable at this endpoint; the credential is. The dashboard session is now required unconditionally, and the refusal names the agent markers only when there are any. The former "hand-typed run stars over HTTP" test encoded exactly the hole, so it is replaced by its inverse plus a dashboard-click case on the same clean environment. The eager producer honored the abort signal before examining a settled read. A read can settle with a real chunk in the same tick the signal fires — the post-cancel drain does exactly this: the terminal frame arrives, then the drain deadline aborts upstream — so the terminal was discarded and the turn was accounted as a cancel. The chunk is inspected first now, and abort is honored immediately after. Driven red by restoring the old order. --- src/server/management/sidebar-routes.ts | 49 +++++++++++++------------ src/server/relay-eager.ts | 9 ++++- tests/relay-eager.test.ts | 22 +++++++++++ tests/sidebar-routes.test.ts | 24 +++++++++++- tests/startup-prompt.test.ts | 9 +++-- 5 files changed, 82 insertions(+), 31 deletions(-) diff --git a/src/server/management/sidebar-routes.ts b/src/server/management/sidebar-routes.ts index 2ebc11d5f..4bc93f1ff 100644 --- a/src/server/management/sidebar-routes.ts +++ b/src/server/management/sidebar-routes.ts @@ -9,20 +9,22 @@ * this surface only learns the yes/no answer. `gh` writes the authenticated account * name to stderr, so that output is discarded at the source rather than forwarded. * - * The star POST additionally refuses agent-driven programmatic callers. Management - * auth proves the caller reached the admin token, not that a person chose to star: - * a coding agent runs on the user's machine and can read that token from disk, so - * the CLI's "ask the user" deferral would be bypassable with one `curl` here. + * The star POST additionally requires a dashboard session. Management auth proves + * the caller reached the admin token, not that a person chose to star: a coding + * agent runs on the user's machine and can read that token from disk, so the CLI's + * "ask the user" deferral would be bypassable with one `curl` here. * - * The dashboard button must keep working even when the proxy itself was started by - * an agent, which is the common case — the person is at the browser, not at the - * spawning shell. A GUI click is therefore distinguished by the CREDENTIAL that - * authorized the request — a GUI session this process minted for a browser, which - * the auth gate only accepts after matching origin and the per-session CSRF token — - * rather than by the proxy's own env or by request headers. + * The requirement is unconditional, and that is the point. It used to apply only + * when `isAgentDriven()` was true — but that function reads the SERVER's + * environment, not the caller's, so a proxy already running as a service (no agent + * markers, the normal remote setup) accepted a raw-token star from anyone who could + * read the token, which includes every agent on the machine. The provenance of the + * HTTP caller is not knowable from the server's env; only the credential is. So the + * mutation asks for a GUI session this process minted for a browser, which the auth + * gate accepts only after matching origin and the per-session CSRF token. */ import { jsonResponse } from "../auth-cors"; -import { agentDrivenMarkers, isAgentDriven } from "../../cli/agent-driven"; +import { agentDrivenMarkers } from "../../cli/agent-driven"; import type { ManagementContext } from "./context"; /** @@ -40,12 +42,11 @@ function hasBrowserSessionEvidence(ctx: ManagementContext): boolean { return ctx.principal === "gui-session"; } -// Known edge, deliberately fail-closed: a non-loopback operator dashboard signs in -// with the raw admin token instead of a minted GUI session, so its clicks carry no -// CSRF header. If that proxy was *also* started from an agent shell, the button is -// refused and the response names the one-line `gh` command to run by hand. A -// service-run proxy (`OCX_SERVICE`, the usual remote setup) is not agent-driven and -// is unaffected. +// Known edge, deliberately fail-closed: a non-loopback operator dashboard that signs +// in with the raw admin token instead of a minted GUI session gets its click refused, +// and the response names the one-line `gh` command to run by hand. That is the +// correct trade — an endpoint reachable with a readable token cannot establish that +// a human chose to spend their own GitHub identity. export async function handleSidebarRoutes(ctx: ManagementContext): Promise { const { req, url } = ctx; @@ -58,10 +59,11 @@ export async function handleSidebarRoutes(ctx: ManagementContext): Promise): Promise { } describe("relaySseEagerBounded — inline payload rewrite (#864)", () => { + test("a terminal frame settling in the same tick as abort is still recorded", async () => { + // Post-cancel drain: the terminal arrives, and the drain deadline aborts + // upstream in the same tick. Honoring the signal before examining the settled + // read discarded that frame, so the turn was accounted as a cancel instead of + // the completion it actually reached. + const up = controlledUpstream(); + const { hooks, rec } = makeHooks(); + const upstream = new AbortController(); + const relayed = relaySseEagerBounded(up.stream, upstream, hooks); + const reading = readAll(relayed); + + up.push(sse(DELTA)); + await settle(); + // Enqueue the terminal and abort without yielding in between. + up.push(enc.encode(`event: response.completed\ndata: ${COMPLETED}\n\n`)); + upstream.abort(new Error("drain window expired")); + up.close(); + await reading; + + expect(rec.terminals.map(t => t.status)).toContain("completed"); + }); + test("rewrites complete blocks across fragmented chunks and flushes the tail at EOF", async () => { const up = controlledUpstream(); const { hooks } = makeHooks(); diff --git a/tests/sidebar-routes.test.ts b/tests/sidebar-routes.test.ts index 3667c4257..4a311e112 100644 --- a/tests/sidebar-routes.test.ts +++ b/tests/sidebar-routes.test.ts @@ -240,14 +240,34 @@ describe("route surface", () => { expect(calls).toEqual([]); }); - test("a hand-typed run is not blocked by the agent guard", async () => { + test("a clean-environment server still refuses a raw-token star", async () => { + // The guard used to fire only when isAgentDriven() was true — but that reads + // the SERVER's environment, not the caller's. A proxy running as a service has + // no agent markers, so this exact request (raw admin token, no dashboard + // session) starred the repository for anyone who could read the token, which + // is every agent on the machine. Caller provenance is not knowable here; the + // credential is, so the dashboard session is required unconditionally. const calls: string[][] = []; await withEnv(NO_AGENT_ENV, () => withStarDeps({ nowMs: () => 0, async runGh(args) { calls.push(args); return { status: 0 }; }, }, async () => { invalidateStarStatusCache(); - const { status } = await call("POST", "/api/github/star"); + const { status, body } = await call("POST", "/api/github/star", {}, "admin-token"); + expect(status).toBe(403); + expect((body as Record).code).toBe("agent_consent_required"); + })); + expect(calls).toEqual([]); + }); + + test("a dashboard click on a clean-environment server still stars", async () => { + const calls: string[][] = []; + await withEnv(NO_AGENT_ENV, () => withStarDeps({ + nowMs: () => 0, + async runGh(args) { calls.push(args); return { status: 0 }; }, + }, async () => { + invalidateStarStatusCache(); + const { status } = await call("POST", "/api/github/star", {}, "gui-session"); expect(status).toBe(200); })); expect(calls.some(args => args.includes("PUT"))).toBe(true); diff --git a/tests/startup-prompt.test.ts b/tests/startup-prompt.test.ts index 529fb8ef4..8674124b8 100644 --- a/tests/startup-prompt.test.ts +++ b/tests/startup-prompt.test.ts @@ -115,12 +115,13 @@ describe("startup star prompt", () => { // The CLI deferral is worthless if an agent can reach the same write over // HTTP: it runs on the user's machine and can read the admin token from disk. - expect(routes).toContain("isAgentDriven()"); expect(routes).toContain("agent_consent_required"); - // A dashboard click must still work when an agent started the proxy, so the - // refusal is conditioned on the absence of browser-session evidence. expect(routes).toContain("hasBrowserSessionEvidence"); - expect(routes).toMatch(/isAgentDriven\(\)\s*&&\s*!hasBrowserSessionEvidence\(ctx\)/); + // The requirement is UNCONDITIONAL. Gating it on isAgentDriven() reads the + // server's environment, not the caller's, so a service-run proxy (no agent + // markers) accepted a raw-token star from any agent on the machine. + expect(routes).toMatch(/if\s*\(!hasBrowserSessionEvidence\(ctx\)\)/); + expect(routes).not.toMatch(/isAgentDriven\(\)\s*&&/); // And that evidence must be the authenticating CREDENTIAL, never a request // header: the admin token is readable by anything running as the user, so a // header-shaped check is forgeable by the exact caller this guard refuses. From cec481cc249e2fae87c2384c9cad8e65ee2044d1 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 11:12:28 +0900 Subject: [PATCH 7/8] test: make the two retention repairs observable instead of merely correct MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The audit's second round showed both resource fixes were mutation-surviving: restoring the per-chunk accumulator, or the shared Promise.race companion, left every test green. Neither property is visible through behavior — both shapes relay and reassemble identically — so each needed its own observable. bounded-body now counts buffer reallocations for tests. A geometric buffer grows a handful of times regardless of how the peer fragments the body; an exact-fit accumulator grows once per chunk, which is the retention shape the repair removed. The new test compares 20k one-byte chunks against the same body in one chunk and pins growth to a small constant. The eager relay's property is structural, so it is pinned structurally, the way this repository already pins the star-consent guard: neither relay may race a read against a shared abort promise (comments stripped first — both files describe the banned shape in prose), and the eager producer must keep the reader-cancel wake-up that replaced it. Both driven red by restoring the old implementations. --- src/lib/bounded-body.ts | 15 +++++++++++++++ tests/bounded-body.test.ts | 27 +++++++++++++++++++++++++++ tests/relay-eager.test.ts | 23 +++++++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/src/lib/bounded-body.ts b/src/lib/bounded-body.ts index 3ff852246..745833c79 100644 --- a/src/lib/bounded-body.ts +++ b/src/lib/bounded-body.ts @@ -39,6 +39,19 @@ export interface BoundedBodyResult { const TOTAL_TIMEOUT = Symbol("bounded body total timeout"); const INACTIVITY_TIMEOUT = Symbol("bounded body inactivity timeout"); +/** + * Test-only instrumentation: how many times the retained buffer was reallocated + * during the most recent read. The accumulator grows geometrically, so this is + * logarithmic in the body size and independent of how many chunks the peer sends. + * The per-chunk array it replaced retained one object per chunk instead, which a + * fragmenting peer can inflate far past the payload ceiling — a property no + * correctness assertion can see, which is why it is observable here. + */ +let bufferGrowthsForTests = 0; +export function boundedBodyBufferGrowthsForTests(): number { + return bufferGrowthsForTests; +} + function timeoutPromise(ms: number, value: symbol): { promise: Promise; clear: () => void } { let timer: ReturnType | undefined; const promise = new Promise((resolve) => { @@ -105,6 +118,7 @@ export async function readBoundedResponseBody( // beyond the payload ceiling on large budgets. let retained = new Uint8Array(Math.min(maxBytes, 64 * 1024)); let retainedBytes = 0; + bufferGrowthsForTests = 0; let mustCancel = false; let cancelReason: unknown; const total = timeoutPromise(options.totalTimeoutMs ?? BOUNDED_BODY_TIMEOUT_MS, TOTAL_TIMEOUT); @@ -197,6 +211,7 @@ export async function readBoundedResponseBody( ); grown.set(retained.subarray(0, retainedBytes)); retained = grown; + bufferGrowthsForTests += 1; } retained.set(value, retainedBytes); retainedBytes += value.byteLength; diff --git a/tests/bounded-body.test.ts b/tests/bounded-body.test.ts index 18989bd1b..481feed46 100644 --- a/tests/bounded-body.test.ts +++ b/tests/bounded-body.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { BOUNDED_BODY_MAX_BYTES, + boundedBodyBufferGrowthsForTests, readBoundedResponseBody, } from "../src/lib/bounded-body"; @@ -162,6 +163,32 @@ describe("readBoundedResponseBody", () => { expect(result.text).toBe("a".repeat(chunkCount)); expect(result.oversized).toBe(false); }); + + test("retention is logarithmic in the body, not linear in the chunk count", async () => { + // The accumulator is the security property, and correctness cannot see it: + // a per-chunk array reassembles identically while retaining one object per + // transport chunk, which a fragmenting peer inflates far past the payload + // ceiling. Growth count is the observable that separates the two — a single + // geometric buffer doubles a handful of times regardless of fragmentation. + const fine = Array.from({ length: 20_000 }, () => new Uint8Array([0x61])); + await readBoundedResponseBody(responseFromChunks(...fine), { maxBytes: CUSTOM_CAP }); + const fineGrowths = boundedBodyBufferGrowthsForTests(); + + const coarse = [new Uint8Array(20_000).fill(0x61)]; + await readBoundedResponseBody(responseFromChunks(...coarse), { maxBytes: CUSTOM_CAP }); + const coarseGrowths = boundedBodyBufferGrowthsForTests(); + + // 20k one-byte chunks fit inside the 64 KiB seed: no growth at all, and the + // same body delivered as one chunk behaves identically. + expect(fineGrowths).toBe(coarseGrowths); + expect(fineGrowths).toBeLessThanOrEqual(2); + + // Past the seed, growth stays logarithmic: doubling from 64 KiB to 256 KiB is + // two reallocations no matter how the peer fragments it. + const big = Array.from({ length: 256 }, () => new Uint8Array(1024).fill(0x61)); + await readBoundedResponseBody(responseFromChunks(...big), { maxBytes: CUSTOM_CAP }); + expect(boundedBodyBufferGrowthsForTests()).toBeLessThanOrEqual(4); + }); }); test("parent abort rejects with the exact reason object", async () => { diff --git a/tests/relay-eager.test.ts b/tests/relay-eager.test.ts index ee8e15451..a66b759d1 100644 --- a/tests/relay-eager.test.ts +++ b/tests/relay-eager.test.ts @@ -111,6 +111,29 @@ async function readAll(stream: ReadableStream): Promise { } describe("relaySseEagerBounded — inline payload rewrite (#864)", () => { + test("neither relay races reads against a shared abort promise", async () => { + // Retention shape, not behavior: racing every read against ONE never-settled + // promise attaches a reaction per completed read and holds it until abort, so a + // long stream retains O(chunk-count) callbacks. Both relays relay identically + // either way, which is exactly why no behavioral assertion catches a regression + // here — relay.ts already states the rule in prose at its own drain, and this + // pins it for both files. The sanctioned shape is: cancel the reader on abort. + const eager = await Bun.file(new URL("../src/server/relay-eager.ts", import.meta.url)).text(); + const relay = await Bun.file(new URL("../src/server/relay.ts", import.meta.url)).text(); + + // Strip comments first: both files DESCRIBE the banned shape in prose, and the + // rule is about the code, not the explanation of why the code avoids it. + const stripComments = (source: string): string => + source.replace(/\/\*[\s\S]*?\*\//g, "").replace(/(^|[^:])\/\/.*$/gm, "$1"); + + for (const [name, source] of [["relay-eager.ts", eager], ["relay.ts", relay]] as const) { + const racesReads = /Promise\.race\(\s*\[\s*reader\.read\(\)/.test(stripComments(source)); + expect(`${name} races reads: ${racesReads}`).toBe(`${name} races reads: false`); + } + // And the eager producer must keep the reader-cancel wake-up that replaced it. + expect(eager).toMatch(/reader\.cancel\(upstream\.signal\.reason\)/); + }); + test("a terminal frame settling in the same tick as abort is still recorded", async () => { // Post-cancel drain: the terminal arrives, and the drain deadline aborts // upstream in the same tick. Honoring the signal before examining the settled From 65429705b1582269d260e00fe6efebc835abc839 Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Mon, 3 Aug 2026 11:19:23 +0900 Subject: [PATCH 8/8] docs(consent), test(bounded-body): state the guard's real limit, pin the shape MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two round-3 audit findings, one accepted as a documentation fix and one as a test-instrument fix. A local process running as the user can mint its own GUI session — the dashboard bootstrap is served to any loopback GET — and can equally run `gh api -X PUT /user/starred/...` without involving the proxy at all. No check inside this process distinguishes that caller from the browser, because both hold every local credential. Claiming the endpoint is a technical barrier would be false, so the route comment and AGENTS.md now say what it actually does: it removes the casual path and the raw-token path, and the real boundary is the normative rule that an agent must not spend the user's identity by any mechanism. The unconditional session requirement stays; it is just no longer described as more than it is. The bounded-body growth counter caught an exact-fit reallocation mutation but not the per-chunk `Uint8Array[]` it replaced, because that implementation never increments the counter at all. The retained-object shape is now pinned structurally — no per-chunk collection, one geometric buffer — the same instrument this repository already uses for the relay retention rule. Driven red by restoring the array accumulator. --- AGENTS.md | 10 +++++++++ src/server/management/sidebar-routes.ts | 10 +++++++++ tests/bounded-body.test.ts | 28 ++++++++++++++++++++----- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 5911d4b97..7f4fcfbd3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -130,6 +130,16 @@ with regression coverage in `tests/startup-prompt.test.ts`, another action that spends the user's identity, credits, or reputation, gate it the same way rather than relying on a prompt an agent can answer. +**Be clear about what that enforcement is and is not.** The management endpoint +requires a dashboard session, which stops the casual path — an agent that would +have POSTed there because the endpoint existed, and one holding only the admin +token. It is not a technical barrier against a determined local agent: a process +running as the user can mint its own session from the loopback dashboard +bootstrap, and can skip the proxy entirely by running `gh` itself. Every local +credential is equally reachable by both the browser and the agent, so no check +inside this process can tell them apart. The real boundary is the rule above, and +it binds you regardless of which mechanism is within reach. + ## Commands ```bash diff --git a/src/server/management/sidebar-routes.ts b/src/server/management/sidebar-routes.ts index 4bc93f1ff..8eb50e1c8 100644 --- a/src/server/management/sidebar-routes.ts +++ b/src/server/management/sidebar-routes.ts @@ -47,6 +47,16 @@ function hasBrowserSessionEvidence(ctx: ManagementContext): boolean { // and the response names the one-line `gh` command to run by hand. That is the // correct trade — an endpoint reachable with a readable token cannot establish that // a human chose to spend their own GitHub identity. +// +// The honest limit of this guard: a local process running AS THE USER can mint its +// own GUI session (the dashboard bootstrap is served to any loopback GET) and can +// equally just run `gh api -X PUT /user/starred/...` itself, which needs no proxy at +// all. No check inside this process can distinguish that caller from the browser, +// because both hold every local credential. So this endpoint is not a technical +// barrier against a determined local agent — it removes the CASUAL path (an agent +// that would have POSTed here because the endpoint existed) and makes the refusal +// legible. The actual boundary is normative and lives in AGENTS.md: an agent must +// not spend the user's identity, whichever mechanism is at hand. export async function handleSidebarRoutes(ctx: ManagementContext): Promise { const { req, url } = ctx; diff --git a/tests/bounded-body.test.ts b/tests/bounded-body.test.ts index 481feed46..41584c643 100644 --- a/tests/bounded-body.test.ts +++ b/tests/bounded-body.test.ts @@ -165,11 +165,10 @@ describe("readBoundedResponseBody", () => { }); test("retention is logarithmic in the body, not linear in the chunk count", async () => { - // The accumulator is the security property, and correctness cannot see it: - // a per-chunk array reassembles identically while retaining one object per - // transport chunk, which a fragmenting peer inflates far past the payload - // ceiling. Growth count is the observable that separates the two — a single - // geometric buffer doubles a handful of times regardless of fragmentation. + // Growth accounting for the single buffer: it doubles a handful of times no + // matter how the peer fragments the body. This catches an exact-fit + // reallocation mutation; the per-chunk ARRAY shape is caught structurally in + // the test below, because that implementation never touches this counter. const fine = Array.from({ length: 20_000 }, () => new Uint8Array([0x61])); await readBoundedResponseBody(responseFromChunks(...fine), { maxBytes: CUSTOM_CAP }); const fineGrowths = boundedBodyBufferGrowthsForTests(); @@ -189,6 +188,25 @@ describe("readBoundedResponseBody", () => { await readBoundedResponseBody(responseFromChunks(...big), { maxBytes: CUSTOM_CAP }); expect(boundedBodyBufferGrowthsForTests()).toBeLessThanOrEqual(4); }); + + test("the accumulator never retains one object per transport chunk", async () => { + // The retained-object shape is the actual security property and no behavioral + // assertion can see it: a `Uint8Array[]` of chunks reassembles byte-identically + // while holding one reference per chunk, which a fragmenting peer inflates far + // past the payload ceiling. It also never increments the growth counter above, + // so that test alone cannot catch it. Pin the shape, the same instrument this + // repository uses for the relay retention rule and the star-consent guard. + const source = (await Bun.file(new URL("../src/lib/bounded-body.ts", import.meta.url)).text()) + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/(^|[^:])\/\/.*$/gm, "$1"); + + // No per-chunk collection: the reader must accumulate into one buffer. + expect(source).not.toMatch(/chunks\s*\.\s*push\s*\(/); + expect(source).not.toMatch(/const\s+chunks\s*:\s*Uint8Array\[\]/); + // And that buffer must be the geometric one this module documents. + expect(source).toMatch(/let\s+retained\s*=\s*new\s+Uint8Array\(/); + expect(source).toMatch(/retained\.set\(value,\s*retainedBytes\)/); + }); }); test("parent abort rejects with the exact reason object", async () => {