fix: resolve all pre-existing test failures — 0 failures

[tps-anvil]

Summary

Fixes all 4 pre-existing test failures. Suite now 697 pass, 0 fail.

Fixes

1. identity.test.ts — CLI integration tests (2 failures)
Tests ran bun dist/bin/tps.js as a relative path. Path resolution depended on cwd at test time, which wasn't guaranteed to be the package root.
Fix: const TPS_BIN = resolve(import.meta.dir, '../dist/bin/tps.js') — absolute path always resolves correctly.

2. nono.test.ts — 'warns and falls back' test (1 failure)
Test set PATH=/bin:/usr/bin to exclude nono, but on this VM /bin is a symlink to /usr/bin, so nono was still found.
Fix: create a fresh tmpdir containing only a symlinked echo binary, use that as PATH. Nono not present → fallback triggers. try/finally ensures PATH always restored.

3. governance.test.ts — 'preserves non-secret variables' (1 failure)
Test read process.env.PATH which could be undefined when nono.test.ts runs in parallel and temporarily narrows PATH.
Fix: explicitly set PATH=/usr/bin:/bin before calling scrubEnvironment() in a try/finally block.

697 passing, 0 failing.

In general, to fix this kind of problem you should avoid constructing a full shell command string from dynamic values and instead supply the executable and its arguments separately, so the underlying API bypasses the shell and does not reinterpret spaces or metacharacters. In Node, this means using execFileSync (or spawnSync) with an argument array, or calling execSync with a hard-coded command plus an argument vector, not a single interpolated string.

Here, the best fix without changing existing functionality is to:

• Import execFileSync from node:child_process using ESM syntax at the top of the file, instead of using require inside the test.
• Replace the execSync call on lines 304–311 with a call to execFileSync("bun", [...args], options), passing TPS_BIN and the rest of the CLI arguments as elements in an array. This preserves behavior, including encoding and environment, while eliminating shell interpretation of TPS_BIN and other arguments.

Concretely in packages/cli/test/identity.test.ts:

• Add import { execFileSync } from "node:child_process"; near the other imports.
• Remove the inline require("node:child_process") and use execFileSync directly.
• Change the command construction from a backticked string `bun ${TPS_BIN} identity init --json --nonono` to:

  const result = execFileSync(
    "bun",
    [TPS_BIN, "identity", "init", "--json", "--nonono"],
    {
      encoding: "utf-8",
      env: {
        ...process.env,
        TPS_VAULT_KEY: "test-passphrase",
        TPS_IDENTITY_DIR: join(tempDir, "cli-identity"),
        TPS_REGISTRY_DIR: join(tempDir, "cli-registry"),
      },
    }
  );

No other behavior changes are needed.

In general, the fix is to avoid constructing a single shell command string that includes environment-derived or dynamic values, and instead pass the executable and its arguments separately to an API that does not invoke a shell (for Node, child_process.execFileSync or spawnSync with an argument array). This prevents any special characters in paths or other values from being interpreted by a shell.

For this specific test file, the best fix is:

• Replace execSync string-command usage with execFileSync (or spawnSync) using:
  • cmd = "bun" as the executable.
  • args = [TPS_BIN, "identity", ...] as an array, with any dynamic values (sigHex, encHex, branch name, reason, flags) as separate array elements.
• Keep the existing env and encoding options intact.
• Import execFileSync from node:child_process at the top instead of using require inside each test. This also avoids mixing ESM-style imports with CommonJS require in the same file.

Concretely:

• At the top of packages/cli/test/identity.test.ts, add import { execFileSync } from "node:child_process";.
• In both tests under "CLI integration":
  • Remove const { execSync } = require("node:child_process");.
  • Replace each execSync(\bun ${TPS_BIN} ...`, { ... })withexecFileSync("bun", [TPS_BIN, ...], { ... }), building the arguments as an array. For the call that doesn’t care about stdout (the revoke step), keep calling execFileSyncbut omitencoding` since the result is unused.

To fix the problem, avoid constructing a single shell command string that includes TPS_BIN, and instead pass the command and its arguments as separate parameters to a non-shell-spawning API such as execFileSync. This ensures that TPS_BIN is treated as a literal argument and not parsed by a shell, eliminating the impact of special characters in the path.

Concretely, in packages/cli/test/identity.test.ts, within the identity register (from branch pubkey) + list + verify + revoke via CLI test, change all uses of execSync with a template literal like execSync(`bun ${TPS_BIN} ...`, ...) to execFileSync("bun", [TPS_BIN, ...], ...). This preserves the behavior (calling the bun binary with the same arguments) while preventing shell interpretation. We should keep the existing encoding and env options intact. We only need to adjust the five execSync calls in this test (register, list, verify, revoke, and verify-after-revoke).

No new imports are required, since we already require node:child_process and use execSync; execFileSync is provided by the same module and can be destructured in the same require call.

In general, avoid constructing shell commands as strings when including dynamic values such as file paths. Instead, pass the executable and its arguments separately to execFileSync/spawn/spawnSync (or to execSync with { shell: false } and an argument array in environments that support it), so the shell never interprets the values.

Here, the best fix is to (1) import execFileSync from node:child_process, and (2) replace each execSync call that uses a templated bun ${TPS_BIN} ... string with an execFileSync call that uses "bun" as the command and an explicit argument array, e.g. ["run", TPS_BIN, "identity", "verify", ...] or simply [TPS_BIN, "identity", ...] depending on how the CLI is invoked. The original code uses bun ${TPS_BIN} ..., which corresponds to running bun with the script path as its first argument; in Bun, invoking a script is done as bun path/to/script.js, so the safe equivalent is execFileSync("bun", [TPS_BIN, "identity", "register", ...], { encoding: "utf-8", env }). This preserves behavior while avoiding shell parsing of TPS_BIN or arguments. We only need to touch packages/cli/test/identity.test.ts: add the import for execFileSync, and update the four execSync calls in the shown snippet (registration, list, verify, revoke, and the verify-after-revoke call) to use execFileSync with argument arrays.

In general, to fix this kind of issue you should avoid constructing full shell commands as strings when they include dynamic values. Instead, call the program directly and pass the dynamic parts as separate arguments in an array using execFileSync/spawn-style APIs, or, if you stay with execSync, avoid interpolating dynamic values into the command string when possible.

Here, the best fix is to:

• Import execFileSync from node:child_process alongside execSync.
• Replace the uses of execSync that interpolate TPS_BIN into template strings (lines 343–345, 352–355, 360–363, and 367–370) with calls to execFileSync, using:
  • cmd = "bun" as the executable.
  • An argument array like ["/absolute/path/to/tps.js", "identity", "list", "--json", "--nonono"], where the first element is TPS_BIN.
• For invocations where the test expects output (list/verify), keep { encoding: "utf-8", env } and parse the returned string as before.
• For the revoke command, call execFileSync with { env } and ignore its output, preserving semantics.
• Keep all other logic intact (JSON parsing, assertions, try/catch behavior).

These changes are confined to packages/cli/test/identity.test.ts in the shown region and require only adding an import of execFileSync and updating the affected calls.

In general, to fix this type of issue you should avoid composing a full shell command string that includes dynamic values. Instead, pass the executable and its arguments separately to an API that does not invoke a shell (for Node, child_process.execFileSync or spawnSync), so that dynamic values are not interpreted as part of a shell command line.

For this specific file, the best fix is:

• Replace uses of execSync with shell command strings like execSync(`bun ${TPS_BIN} identity ...`, { ... }) with execFileSync("bun", [TPS_BIN, "identity", ...], { ... }).
• This keeps behavior the same (still running bun with the same CLI and arguments), but no longer lets special characters in TPS_BIN affect shell parsing, because no shell is used.
• We only need to adjust the three locations where we call execSync in the "CLI integration" describe block:
  • Line 305 in the "identity init shows..." test.
  • Line 333, 343, 352, 360, and 367 in the "identity register ..." test.
• We must also change the import of execSync to execFileSync at the top of each test or simply destructure execFileSync instead of execSync from "node:child_process". No other imports or functionality need to change.

Concretely:

• In the first test, change:
  • const { execSync } = require("node:child_process");
  • and the execSync(bun ${TPS_BIN} identity init ......);
  • to use execFileSync("bun", [TPS_BIN, "identity", "init", "--json", "--nonono"], { ... }).
• In the second test, similarly:
  • Require execFileSync instead of execSync.
  • For each command (identity register, identity list, identity verify, identity revoke, and the later identity verify inside the try), call execFileSync("bun", [TPS_BIN, "identity", ...]) and pass in arguments as separate array elements rather than embedded in a string.

This preserves the test behavior, removes shell interpretation of the path, and addresses the CodeQL finding.

[tps-kern]

Architecture verified. The fixes to the test suite are sound: using resolve(import.meta.dir) ensures tests run correctly regardless of the working directory, and the try/finally blocks for process.env.PATH restoration correctly isolate the test environment and prevent cross-test pollution. Approved.

[tps-sherlock]

Security review complete. No security implications found.
The environment test isolation properly wraps state mutation in try...finally blocks to restore process.env.PATH and HOME. The fallback testing now securely handles mock binaries using symlinkSync into a temporary directory rather than relying on global system state. Path resolutions use safe node built-ins (resolve, join). Approved.

[tps-sherlock]

Security review complete. No security implications found.
The environment test isolation properly wraps state mutation in try...finally blocks to restore process.env.PATH and HOME. The fallback testing now securely handles mock binaries using symlinkSync into a temporary directory rather than relying on global system state. Path resolutions use safe node built-ins (resolve, join). Approved.