sdk/node: reject arrays for transform and transform.headers

[dwin-gharibi]

Closes #1468.

Motivation

convertE2BTransformToInject validated with typeof headers !== "object". In JavaScript
typeof [] === "object", so an array passed the guard, and Object.entries(["a","b"]) produced
[["0","a"],["1","b"]] — injects whose header names are the array indices "0" and "1", carrying real
credentials.

The Python SDK rejects the same input via isinstance(headers, dict), so this was also a cross-SDK
divergence. And the server does not catch it: validate_policy requires only a non-empty string header
name, and "0" satisfies that, so the malformed policy is accepted end to end.

What this changes

sdk/node/src/policy.ts — two guards gain an Array.isArray check:

if (typeof transform !== "object" || transform === null || Array.isArray(transform)) { ... }
if (typeof headers !== "object" || headers === null || Array.isArray(headers)) { ... }

Error messages are unchanged, so callers matching on them are unaffected. Strings and numbers were
already rejected; this closes the array case, and does the same for the outer transform for
consistency.

No comment changes.

Testing

New: sdk/node/test/policy-e2b-shapes.test.ts

• rejects an array of header values (the reported case)
• rejects a string, rejects a number (regression guards for the paths that already worked)
• rejects an array transform
• accepts a plain header map, and accepts multiple headers in one transform with order preserved

Red/green — with policy.ts reverted:

× convertE2BPerHostRules transform.headers validation > rejects an array of header values
  Tests  1 failed | 5 passed (6)

with the fix, and the whole SDK suite:

$ npx vitest run
Test Files  16 passed | 1 skipped (17)
     Tests  192 passed | 1 skipped (193)

CI gates checked locally:

• npx tsc --noEmit — clean.
• npx vitest run — 192 passed (sdk-test-check).

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubesandboxbot]

This test doesn't exercise the new Array.isArray(transform) guard. With input { "api.example.com": [["headers"]] }, entries is [["headers"]] and entry is the array ["headers"] — the entry object check (typeof entry !== "object" || entry === null) passes arrays through, so the code path taken is (entry as E2BTransformEntry).transform → undefined → the pre-existing "missing the 'transform' field" error thrown in convertE2BPerHostRules. convertE2BTransformToInject is never reached, and /transform/ happens to match both messages.

That's why the red/green run shows only the headers-array test going red — this test passes before and after the fix. To actually cover the new branch, pass a transform that is an array but still reaches convertE2BTransformToInject, e.g.:

it("rejects an array transform", () => {
  expect(() =>
    convertE2BPerHostRules({ "api.example.com": [{ transform: ["headers"] }] } as never),
  ).toThrow(/transform must be an object/);
});

With the current as never/array-input approach, consider dropping the test or asserting the specific message. Note that even the corrected input only turns red when the regex is specific — a bare-array transform without a .headers property throws "requires a 'headers' field" in the unfixed code, so the guard's distinctive observable outcome is the message, not the throw.

[cubesandboxbot]

Review — PR #1469: sdk/node: reject arrays for transform and transform.headers

AI-generated review. No human approval claimed.

Overview

Fixes #1468. convertE2BTransformToInject validated with typeof headers !== "object", and since typeof [] === "object" an array passed the guard. Object.entries(["sk-1", "sk-2"]) then produced [["0", "sk-1"], ["1", "sk-2"]] — injects whose header names are the array indices "0"/"1", carrying real credentials. The fix adds Array.isArray to both the transform and transform.headers guards, closing the leak and matching the Python SDK, which rejects the same input via isinstance(headers, dict) (sdk/python/cubesandbox/_policy.py:297). Error message text is unchanged, so callers matching on messages are unaffected.

Verdict: approve

The fix is correct, minimal, and consistent with the sibling SDK. The core reported bug (headers array → index-key injects) is genuinely closed: tracing the input perHost(["sk-secret-1", "sk-secret-2"]), the unfixed code reaches Object.entries(["sk-secret-1","sk-secret-2"]) and returns 2 injects without throwing; with the fix it throws transform.headers must be an object. The primary regression test is a real red/green. The Array.isArray(transform) guard is defense-in-depth (a bare array transform throws "requires a 'headers' field" anyway, since arrays lack .headers), but it produces a more accurate error and is harmless.

Finding (minor)

1. rejects an array transform test is a false positive — sdk/node/test/policy-e2b-shapes.test.ts:27-31. Input { "api.example.com": [["headers"]] } never reaches the new guard: entry is the array ["headers"], the entry object check passes arrays through, .transform resolves to undefined, and the pre-existing "missing the 'transform' field" error fires in convertE2BPerHostRules. /transform/ matches both messages, so the test passes before and after the fix (consistent with the PR's own red/green run, where only the headers-array test went red). Recommend input { "api.example.com": [{ transform: ["headers"] }] } with the specific assertion /transform must be an object/. See inline comment.

Nits (no change required)

• The sibling entry object check in convertE2BPerHostRules (typeof entry !== "object" || entry === null, sdk/node/src/policy.ts:238) has the same latent array-passthrough. An array entry still errors in practice (via the missing-transform check) and can't leak credentials without an explicitly-set .transform property, so this is cosmetic — but for consistency with this PR's stated goal it could also gain || Array.isArray(entry).
• The new test file uses as never casts to smuggle untyped shapes into convertE2BPerHostRules; fine for a compat-layer guard test, just noting it doesn't type-check the E2B contract.

Testing claims

The stated checks (tsc clean, 192 passed / 1 skipped) are consistent with the diff and the surrounding suite. No test runs were executed in this review; claims were checked by code-path tracing against the base tree.