diff --git a/src/agent/__tests__/prompt-assembler.test.ts b/src/agent/__tests__/prompt-assembler.test.ts index e3339478..3d24f185 100644 --- a/src/agent/__tests__/prompt-assembler.test.ts +++ b/src/agent/__tests__/prompt-assembler.test.ts @@ -9,7 +9,8 @@ const baseConfig: PhantomConfig = { port: 3100, role: "swe", model: "claude-opus-4-6", - provider: { type: "anthropic" }, + provider: { type: "anthropic", secret_name: "provider_token" }, + secret_source: "env", effort: "max", max_budget_usd: 0, timeout_minutes: 240, diff --git a/src/cli/doctor.ts b/src/cli/doctor.ts index 98bb6dbc..c916e444 100644 --- a/src/cli/doctor.ts +++ b/src/cli/doctor.ts @@ -90,7 +90,7 @@ async function checkConfig(): Promise { } try { const { loadConfig } = await import("../config/loader.ts"); - const config = loadConfig(); + const config = await loadConfig(); return { name: "Config", status: "ok", message: `${config.name} (${config.role}, port ${config.port})` }; } catch (err: unknown) { const msg = err instanceof Error ? err.message : String(err); diff --git a/src/config/__tests__/loader-metadata.test.ts b/src/config/__tests__/loader-metadata.test.ts new file mode 100644 index 00000000..f1cf0433 --- /dev/null +++ b/src/config/__tests__/loader-metadata.test.ts @@ -0,0 +1,179 @@ +import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test"; +import { mkdirSync, rmSync, writeFileSync } from "node:fs"; +import { loadConfig } from "../loader.ts"; + +// Distinct TEST_DIR from loader.test.ts so the two suites cannot race on the +// same filesystem path when bun runs them in parallel. +const TEST_DIR = "/tmp/phantom-test-config-metadata"; + +function writeYaml(filename: string, content: string): string { + mkdirSync(TEST_DIR, { recursive: true }); + const path = `${TEST_DIR}/${filename}`; + writeFileSync(path, content); + return path; +} + +function cleanup(): void { + rmSync(TEST_DIR, { recursive: true, force: true }); +} + +describe("loadConfig secret_source", () => { + const originalFetch = globalThis.fetch; + const savedKey = process.env.ANTHROPIC_API_KEY; + const savedToken = process.env.ANTHROPIC_AUTH_TOKEN; + + beforeEach(() => { + process.env.ANTHROPIC_API_KEY = undefined; + process.env.ANTHROPIC_AUTH_TOKEN = undefined; + }); + + afterEach(() => { + globalThis.fetch = originalFetch; + if (savedKey !== undefined) { + process.env.ANTHROPIC_API_KEY = savedKey; + } else { + process.env.ANTHROPIC_API_KEY = undefined; + } + if (savedToken !== undefined) { + process.env.ANTHROPIC_AUTH_TOKEN = savedToken; + } else { + process.env.ANTHROPIC_AUTH_TOKEN = undefined; + } + cleanup(); + }); + + test("secret_source defaults to 'env' when omitted from YAML", async () => { + const path = writeYaml("default-source.yaml", "name: default-source"); + const config = await loadConfig(path); + expect(config.secret_source).toBe("env"); + expect(config.secret_source_url).toBeUndefined(); + }); + + test("secret_source: metadata populates ANTHROPIC_API_KEY and ANTHROPIC_AUTH_TOKEN from gateway", async () => { + // Body deliberately not asserted as a string literal anywhere; we read it + // back from process.env and compare to the stub-injected reference. + const stubBody = "stub-token-value"; + globalThis.fetch = mock((url: string | Request) => { + expect(String(url)).toBe("http://gateway.test/v1/secrets/provider_token"); + return Promise.resolve( + new Response(stubBody, { + status: 200, + headers: { "X-Phantom-Rotation-Id": "1" }, + }), + ); + }) as unknown as typeof fetch; + + const path = writeYaml( + "metadata-source.yaml", + ` +name: metadata-tenant +secret_source: metadata +secret_source_url: http://gateway.test +`, + ); + await loadConfig(path); + expect(process.env.ANTHROPIC_API_KEY).toBe(stubBody); + expect(process.env.ANTHROPIC_AUTH_TOKEN).toBe(stubBody); + }); + + test("secret_source: metadata resolves whole-string ${secret:NAME} references in nested config", async () => { + // Use peers.test_peer.token, an existing schema field that accepts an + // arbitrary string and is nested. We assert the resolved value via a + // non-logging path: read it back from the returned config object. + const peerSecret = "peer-secret-payload"; + globalThis.fetch = mock((url: string | Request) => { + const u = String(url); + if (u.endsWith("/v1/secrets/provider_token")) { + return Promise.resolve( + new Response("provider-token-value", { status: 200, headers: { "X-Phantom-Rotation-Id": "1" } }), + ); + } + if (u.endsWith("/v1/secrets/peer_token")) { + return Promise.resolve(new Response(peerSecret, { status: 200, headers: { "X-Phantom-Rotation-Id": "1" } })); + } + throw new Error(`unexpected URL in test: ${u}`); + }) as unknown as typeof fetch; + + const path = writeYaml( + "metadata-walker.yaml", + ` +name: walker-tenant +secret_source: metadata +secret_source_url: http://gateway.test +peers: + test_peer: + url: https://peer.test + token: \${secret:peer_token} +`, + ); + const config = await loadConfig(path); + expect(config.peers?.test_peer?.token).toBe(peerSecret); + }); + + test("secret_source: metadata does NOT resolve partial-string ${secret:NAME} (security invariant)", async () => { + // Whole-string-only matching guards against secret leakage via logged + // URLs or composed strings. If this test ever fails, the regex changed + // in a way that allows partial interpolation, which is a security regression. + const partial = "https://peer.test/?token=${secret:peer_token}"; + globalThis.fetch = mock((url: string | Request) => { + const u = String(url); + if (u.endsWith("/v1/secrets/provider_token")) { + return Promise.resolve( + new Response("provider-token-value", { status: 200, headers: { "X-Phantom-Rotation-Id": "1" } }), + ); + } + throw new Error(`unexpected URL in test (partial-string should NOT trigger fetch): ${u}`); + }) as unknown as typeof fetch; + + const path = writeYaml( + "metadata-partial.yaml", + ` +name: partial-tenant +secret_source: metadata +secret_source_url: http://gateway.test +peers: + test_peer: + url: ${partial} + token: keep-as-is +`, + ); + const config = await loadConfig(path); + expect(config.peers?.test_peer?.url).toBe(partial); + expect(config.peers?.test_peer?.token).toBe("keep-as-is"); + }); + + test("secret_source: metadata surfaces fetch failures as errors that include the secret name", async () => { + globalThis.fetch = mock(() => + Promise.resolve(new Response("internal error", { status: 500, statusText: "Internal Server Error" })), + ) as unknown as typeof fetch; + + const path = writeYaml( + "metadata-fail.yaml", + ` +name: fail-tenant +secret_source: metadata +secret_source_url: http://gateway.test +`, + ); + await expect(loadConfig(path)).rejects.toThrow(/provider_token/); + await expect(loadConfig(path)).rejects.toThrow(/500/); + }); + + test("invalid provider.secret_name rejects at loadConfig parse time with a schema error", async () => { + // Phase C #5 review Finding 1: pinning the regex at the schema level + // surfaces bad names at parse time rather than crashing at boot inside + // the metadata fetcher. We use secret_source: env so no fetcher is + // constructed; the rejection must come from PhantomConfigSchema itself. + const path = writeYaml( + "bad-secret-name.yaml", + ` +name: bad-name-tenant +provider: + type: anthropic + secret_name: Bad-Name +`, + ); + await expect(loadConfig(path)).rejects.toThrow(/Invalid config/); + await expect(loadConfig(path)).rejects.toThrow(/secret_name/); + }); +}); diff --git a/src/config/__tests__/loader.test.ts b/src/config/__tests__/loader.test.ts index 1bbc8a12..9ba0c77f 100644 --- a/src/config/__tests__/loader.test.ts +++ b/src/config/__tests__/loader.test.ts @@ -1,6 +1,6 @@ import { describe, expect, test } from "bun:test"; import { mkdirSync, rmSync, writeFileSync } from "node:fs"; -import { loadConfig } from "../loader.ts"; +import { loadConfig, loadConfigSync } from "../loader.ts"; const TEST_DIR = "/tmp/phantom-test-config"; @@ -16,7 +16,7 @@ function cleanup(): void { } describe("loadConfig", () => { - test("loads a valid config file", () => { + test("loads a valid config file", async () => { const path = writeYaml( "valid.yaml", ` @@ -29,7 +29,7 @@ max_budget_usd: 25 `, ); try { - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.name).toBe("test-phantom"); expect(config.port).toBe(3200); expect(config.role).toBe("swe"); @@ -41,7 +41,7 @@ max_budget_usd: 25 } }); - test("applies defaults for optional fields", () => { + test("applies defaults for optional fields", async () => { const path = writeYaml( "minimal.yaml", ` @@ -49,7 +49,7 @@ name: minimal `, ); try { - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.name).toBe("minimal"); expect(config.port).toBe(3100); expect(config.role).toBe("swe"); @@ -60,11 +60,11 @@ name: minimal } }); - test("throws on missing file", () => { - expect(() => loadConfig("/tmp/phantom-nonexistent.yaml")).toThrow("Config file not found"); + test("throws on missing file", async () => { + await expect(loadConfig("/tmp/phantom-nonexistent.yaml")).rejects.toThrow("Config file not found"); }); - test("throws on invalid config", () => { + test("throws on invalid config", async () => { const path = writeYaml( "invalid.yaml", ` @@ -72,13 +72,13 @@ port: -1 `, ); try { - expect(() => loadConfig(path)).toThrow("Invalid config"); + await expect(loadConfig(path)).rejects.toThrow("Invalid config"); } finally { cleanup(); } }); - test("throws on invalid effort value", () => { + test("throws on invalid effort value", async () => { const path = writeYaml( "bad-effort.yaml", ` @@ -87,13 +87,13 @@ effort: turbo `, ); try { - expect(() => loadConfig(path)).toThrow("Invalid config"); + await expect(loadConfig(path)).rejects.toThrow("Invalid config"); } finally { cleanup(); } }); - test("env var overrides YAML model", () => { + test("env var overrides YAML model", async () => { const path = writeYaml( "env-model.yaml", ` @@ -104,7 +104,7 @@ model: claude-opus-4-6 const saved = process.env.PHANTOM_MODEL; try { process.env.PHANTOM_MODEL = "claude-sonnet-4-6"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.model).toBe("claude-sonnet-4-6"); } finally { if (saved !== undefined) { @@ -116,7 +116,7 @@ model: claude-opus-4-6 } }); - test("env var overrides YAML domain", () => { + test("env var overrides YAML domain", async () => { const path = writeYaml( "env-domain.yaml", ` @@ -127,7 +127,7 @@ domain: old.example.com const saved = process.env.PHANTOM_DOMAIN; try { process.env.PHANTOM_DOMAIN = "new.ghostwright.dev"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.domain).toBe("new.ghostwright.dev"); } finally { if (saved !== undefined) { @@ -139,7 +139,7 @@ domain: old.example.com } }); - test("PHANTOM_NAME env var overrides YAML name", () => { + test("PHANTOM_NAME env var overrides YAML name", async () => { const path = writeYaml( "env-name.yaml", ` @@ -149,7 +149,7 @@ name: phantom-dev const saved = process.env.PHANTOM_NAME; try { process.env.PHANTOM_NAME = "cheema"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.name).toBe("cheema"); } finally { if (saved !== undefined) { @@ -161,7 +161,7 @@ name: phantom-dev } }); - test("PHANTOM_NAME env var is trimmed", () => { + test("PHANTOM_NAME env var is trimmed", async () => { const path = writeYaml( "env-name-trim.yaml", ` @@ -171,7 +171,7 @@ name: phantom-dev const saved = process.env.PHANTOM_NAME; try { process.env.PHANTOM_NAME = " cheema "; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.name).toBe("cheema"); } finally { if (saved !== undefined) { @@ -183,7 +183,7 @@ name: phantom-dev } }); - test("empty PHANTOM_NAME env var does not override YAML", () => { + test("empty PHANTOM_NAME env var does not override YAML", async () => { const path = writeYaml( "env-name-empty.yaml", ` @@ -193,7 +193,7 @@ name: phantom-dev const saved = process.env.PHANTOM_NAME; try { process.env.PHANTOM_NAME = ""; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.name).toBe("phantom-dev"); } finally { if (saved !== undefined) { @@ -205,7 +205,7 @@ name: phantom-dev } }); - test("PHANTOM_ROLE env var overrides YAML role", () => { + test("PHANTOM_ROLE env var overrides YAML role", async () => { const path = writeYaml( "env-role.yaml", ` @@ -216,7 +216,7 @@ role: swe const saved = process.env.PHANTOM_ROLE; try { process.env.PHANTOM_ROLE = "base"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.role).toBe("base"); } finally { if (saved !== undefined) { @@ -228,7 +228,7 @@ role: swe } }); - test("PHANTOM_EFFORT env var overrides YAML effort with valid value", () => { + test("PHANTOM_EFFORT env var overrides YAML effort with valid value", async () => { const path = writeYaml( "env-effort.yaml", ` @@ -239,7 +239,7 @@ effort: max const saved = process.env.PHANTOM_EFFORT; try { process.env.PHANTOM_EFFORT = "low"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.effort).toBe("low"); } finally { if (saved !== undefined) { @@ -251,7 +251,7 @@ effort: max } }); - test("PHANTOM_EFFORT env var with invalid value falls back to YAML", () => { + test("PHANTOM_EFFORT env var with invalid value falls back to YAML", async () => { const path = writeYaml( "env-effort-invalid.yaml", ` @@ -262,7 +262,7 @@ effort: high const saved = process.env.PHANTOM_EFFORT; try { process.env.PHANTOM_EFFORT = "turbo"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.effort).toBe("high"); } finally { if (saved !== undefined) { @@ -274,7 +274,7 @@ effort: high } }); - test("PORT env var overrides YAML port", () => { + test("PORT env var overrides YAML port", async () => { const path = writeYaml( "env-port.yaml", ` @@ -285,7 +285,7 @@ port: 3100 const saved = process.env.PORT; try { process.env.PORT = "8080"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.port).toBe(8080); } finally { if (saved !== undefined) { @@ -297,7 +297,7 @@ port: 3100 } }); - test("PORT env var with non-numeric value falls back to YAML", () => { + test("PORT env var with non-numeric value falls back to YAML", async () => { const path = writeYaml( "env-port-nan.yaml", ` @@ -308,7 +308,7 @@ port: 3100 const saved = process.env.PORT; try { process.env.PORT = "abc"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.port).toBe(3100); } finally { if (saved !== undefined) { @@ -320,7 +320,7 @@ port: 3100 } }); - test("PORT env var with out-of-range value falls back to YAML", () => { + test("PORT env var with out-of-range value falls back to YAML", async () => { const path = writeYaml( "env-port-range.yaml", ` @@ -331,7 +331,7 @@ port: 3100 const saved = process.env.PORT; try { process.env.PORT = "70000"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.port).toBe(3100); } finally { if (saved !== undefined) { @@ -343,7 +343,7 @@ port: 3100 } }); - test("defaults provider to anthropic when block is absent", () => { + test("defaults provider to anthropic when block is absent", async () => { const path = writeYaml( "no-provider.yaml", ` @@ -351,7 +351,7 @@ name: test `, ); try { - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.provider.type).toBe("anthropic"); expect(config.provider.base_url).toBeUndefined(); } finally { @@ -359,7 +359,7 @@ name: test } }); - test("loads a zai provider block", () => { + test("loads a zai provider block", async () => { const path = writeYaml( "zai-provider.yaml", ` @@ -372,7 +372,7 @@ provider: `, ); try { - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.provider.type).toBe("zai"); expect(config.provider.api_key_env).toBe("ZAI_API_KEY"); expect(config.provider.model_mappings?.opus).toBe("glm-5.1"); @@ -381,7 +381,7 @@ provider: } }); - test("PHANTOM_PROVIDER_TYPE env var overrides YAML provider.type", () => { + test("PHANTOM_PROVIDER_TYPE env var overrides YAML provider.type", async () => { const path = writeYaml( "env-provider-type.yaml", ` @@ -393,7 +393,7 @@ provider: const saved = process.env.PHANTOM_PROVIDER_TYPE; try { process.env.PHANTOM_PROVIDER_TYPE = "ollama"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.provider.type).toBe("ollama"); } finally { if (saved !== undefined) { @@ -405,7 +405,7 @@ provider: } }); - test("PHANTOM_PROVIDER_TYPE with unknown value leaves YAML provider.type alone", () => { + test("PHANTOM_PROVIDER_TYPE with unknown value leaves YAML provider.type alone", async () => { const path = writeYaml( "env-provider-type-bad.yaml", ` @@ -417,7 +417,7 @@ provider: const saved = process.env.PHANTOM_PROVIDER_TYPE; try { process.env.PHANTOM_PROVIDER_TYPE = "mystery-llm"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.provider.type).toBe("zai"); } finally { if (saved !== undefined) { @@ -429,7 +429,7 @@ provider: } }); - test("PHANTOM_PROVIDER_BASE_URL env var overrides YAML provider.base_url", () => { + test("PHANTOM_PROVIDER_BASE_URL env var overrides YAML provider.base_url", async () => { const path = writeYaml( "env-provider-baseurl.yaml", ` @@ -442,7 +442,7 @@ provider: const saved = process.env.PHANTOM_PROVIDER_BASE_URL; try { process.env.PHANTOM_PROVIDER_BASE_URL = "https://new.example.com/v1"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.provider.base_url).toBe("https://new.example.com/v1"); } finally { if (saved !== undefined) { @@ -454,7 +454,7 @@ provider: } }); - test("PHANTOM_PROVIDER_BASE_URL with malformed URL is ignored", () => { + test("PHANTOM_PROVIDER_BASE_URL with malformed URL is ignored", async () => { const path = writeYaml( "env-provider-baseurl-bad.yaml", ` @@ -467,7 +467,7 @@ provider: const saved = process.env.PHANTOM_PROVIDER_BASE_URL; try { process.env.PHANTOM_PROVIDER_BASE_URL = "not a url"; - const config = loadConfig(path); + const config = await loadConfig(path); expect(config.provider.base_url).toBe("http://old.example.com"); } finally { if (saved !== undefined) { @@ -478,4 +478,20 @@ provider: cleanup(); } }); + + test("loadConfigSync remains synchronous for legacy callers", () => { + const path = writeYaml( + "sync.yaml", + ` +name: sync-test +`, + ); + try { + const config = loadConfigSync(path); + expect(config.name).toBe("sync-test"); + expect(config.secret_source).toBe("env"); + } finally { + cleanup(); + } + }); }); diff --git a/src/config/__tests__/metadata-fetcher.test.ts b/src/config/__tests__/metadata-fetcher.test.ts new file mode 100644 index 00000000..5e524f89 --- /dev/null +++ b/src/config/__tests__/metadata-fetcher.test.ts @@ -0,0 +1,153 @@ +import { afterEach, describe, expect, mock, test } from "bun:test"; +import { METADATA_CACHE_TTL_MS, MetadataSecretFetcher } from "../metadata-fetcher.ts"; + +// Test seam: tweak the module-level TTL constant from a test by re-exporting +// or by stubbing fetchedAt. Bun does not let us mutate const exports, so the +// 304 test simulates expiry by mutating the cached entry's `fetchedAt` via +// a second `get()` after a known-fresh first call. We assert behaviour, not +// internals: the fetch mock is the source of truth on whether a network +// call occurred. + +const originalFetch = globalThis.fetch; + +afterEach(() => { + globalThis.fetch = originalFetch; +}); + +describe("MetadataSecretFetcher", () => { + test("cold cache fetch returns body and stores rotation id", async () => { + const stubBody = "secret-value-cold"; + const fetchMock = mock((url: string | Request) => { + expect(String(url)).toBe("http://gateway.test/v1/secrets/provider_token"); + return Promise.resolve( + new Response(stubBody, { + status: 200, + headers: { "X-Phantom-Rotation-Id": "r1" }, + }), + ); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + const value = await fetcher.get("provider_token"); + expect(value).toBe(stubBody); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("warm cache hit within TTL skips the network", async () => { + const stubBody = "secret-value-warm"; + const fetchMock = mock(() => + Promise.resolve( + new Response(stubBody, { + status: 200, + headers: { "X-Phantom-Rotation-Id": "r1" }, + }), + ), + ); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + const first = await fetcher.get("provider_token"); + const second = await fetcher.get("provider_token"); + expect(first).toBe(stubBody); + expect(second).toBe(stubBody); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + test("304 response refreshes TTL and returns cached value with If-None-Match", async () => { + const stubBody = "secret-value-rotation"; + let callIndex = 0; + const fetchMock = mock((_url: string | Request, init?: RequestInit) => { + callIndex += 1; + if (callIndex === 1) { + return Promise.resolve( + new Response(stubBody, { + status: 200, + headers: { "X-Phantom-Rotation-Id": "r1" }, + }), + ); + } + expect(init?.headers).toEqual({ "If-None-Match": '"r1"' }); + return Promise.resolve(new Response(null, { status: 304 })); + }); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + const first = await fetcher.get("provider_token"); + + // Force expiry by reaching into the private cache via a typed accessor. + // We use the public TTL constant to compute a stale fetchedAt. + const cache = (fetcher as unknown as { cache: Map }).cache; + const entry = cache.get("provider_token"); + if (!entry) throw new Error("test setup: cache entry missing after first fetch"); + entry.fetchedAt = Date.now() - METADATA_CACHE_TTL_MS - 1; + + const second = await fetcher.get("provider_token"); + expect(first).toBe(stubBody); + expect(second).toBe(stubBody); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); + + test("HTTP 500 throws an error containing the status and secret name, never plaintext", async () => { + globalThis.fetch = mock(() => + Promise.resolve(new Response("plaintext-secret-must-not-leak", { status: 500, statusText: "Server Error" })), + ) as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + try { + await fetcher.get("provider_token"); + throw new Error("expected fetcher.get to throw"); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + expect(msg).toContain("provider_token"); + expect(msg).toContain("500"); + expect(msg).not.toContain("plaintext-secret-must-not-leak"); + } + }); + + test("invalid secret name is rejected before any network call", async () => { + const fetchMock = mock(() => Promise.reject(new Error("test setup: fetch should not be called"))); + globalThis.fetch = fetchMock as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + await expect(fetcher.get("Provider Token!")).rejects.toThrow(/invalid secret name/); + expect(fetchMock).toHaveBeenCalledTimes(0); + }); + + test("encodeURIComponent is applied to the secret name in the URL", async () => { + // The validation regex already restricts names to [a-z_][a-z0-9_]*, so + // the URL-encoded form is identical to the input. This test pins the + // behaviour so a future change to the validation regex (e.g. allowing + // dashes) does not silently bypass URL encoding. + const recordedUrls: string[] = []; + globalThis.fetch = mock((url: string | Request) => { + recordedUrls.push(String(url)); + return Promise.resolve(new Response("ok", { status: 200, headers: { "X-Phantom-Rotation-Id": "r1" } })); + }) as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + await fetcher.get("a_secret_with_no_special_chars"); + expect(recordedUrls[0]).toBe("http://gateway.test/v1/secrets/a_secret_with_no_special_chars"); + }); + + test("304 with no cached entry surfaces as an error containing the secret name", async () => { + // 304-without-cache is a server bug. Pin the loud-error behaviour so a + // future refactor that inverted the `if (!cached)` test would fail here. + globalThis.fetch = mock(() => Promise.resolve(new Response(null, { status: 304 }))) as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + await expect(fetcher.get("provider_token")).rejects.toThrow(/304/); + await expect(fetcher.get("provider_token")).rejects.toThrow(/provider_token/); + }); + + test("network error from underlying fetch is wrapped with the secret name", async () => { + // Transport failures (DNS, ECONNREFUSED, TLS) must be wrapped with the + // secret name so the operator can see which fetch call failed without + // leaking response bodies (there is no body on a transport error). + globalThis.fetch = mock(() => Promise.reject(new Error("ECONNREFUSED"))) as unknown as typeof fetch; + + const fetcher = new MetadataSecretFetcher("http://gateway.test"); + await expect(fetcher.get("provider_token")).rejects.toThrow(/provider_token/); + await expect(fetcher.get("provider_token")).rejects.toThrow(/ECONNREFUSED/); + }); +}); diff --git a/src/config/loader.ts b/src/config/loader.ts index 32bfd1e2..1084a28d 100644 --- a/src/config/loader.ts +++ b/src/config/loader.ts @@ -1,13 +1,57 @@ +// Phantom config loader. +// +// Two entry points: `loadConfigSync` (the historical sync function, kept under +// a new name so it remains usable from contexts that legitimately cannot await) +// and `loadConfig` (a thin async wrapper that adds the Phase C metadata-secret +// resolution path on top). For the default `secret_source: "env"` the wrapper +// is sync-fast: it parses, validates, returns. The async machinery only runs +// when an operator opts in via `secret_source: metadata` in phantom.yaml. +// +// When `secret_source === "metadata"` the loader fetches the provider token +// from the host metadata gateway and writes it into `process.env` BEFORE +// returning. This is deliberate: `buildProviderEnv` (in providers.ts) reads +// `process.env.` to populate the Agent SDK subprocess env, and +// keeping that contract means the metadata path is a transparent prefix to +// the existing flow with zero downstream plumbing changes. +// +// Security invariant on `${secret:NAME}` interpolation: the regex requires +// the ENTIRE string value to be `${secret:NAME}`. Partial substring matches +// like `https://hook/?token=${secret:provider_token}` are intentionally NOT +// resolved. If we matched substrings, an operator could embed a secret into +// any URL or log message, and downstream code that logs URLs would leak the +// plaintext. By requiring whole-string matches, the resolved replacement IS +// the plaintext itself, never a string containing the plaintext, so the +// surface for accidental disclosure is the existing `process.env` surface. +// +// Note on type safety: the walker treats the config as a generic +// Record so it can recurse over arbitrary nested objects. +// The single `as unknown as Record` cast at the entry point +// (where loadConfig hands the typed PhantomConfig to interpolateSecretsInPlace) +// is the only place we cross the typed/generic boundary; the walker itself +// does not introduce further casts beyond a recursive descent step. + import { readFileSync } from "node:fs"; import { parse } from "yaml"; +import { MetadataSecretFetcher } from "./metadata-fetcher.ts"; import { PROVIDER_TYPES, type ProviderType } from "./providers.ts"; import { type ChannelsConfig, ChannelsConfigSchema, PhantomConfigSchema } from "./schemas.ts"; import type { PhantomConfig } from "./types.ts"; const DEFAULT_CONFIG_PATH = "config/phantom.yaml"; const DEFAULT_CHANNELS_PATH = "config/channels.yaml"; +const DEFAULT_METADATA_BASE_URL = "http://169.254.169.254"; +const SECRET_REF_REGEX = /^\$\{secret:([a-z_][a-z0-9_]*)\}$/; -export function loadConfig(path?: string): PhantomConfig { +/** + * Synchronous loader. Reads the YAML, validates against the schema, applies + * env-var overrides, and returns the parsed config. Use this from contexts + * that genuinely cannot await; metadata-mode resolution is unavailable here. + * + * The default `secret_source: "env"` path is byte-identical to the loader + * that shipped before Phase C: an operator who never sets `secret_source` + * sees no behaviour change. + */ +export function loadConfigSync(path?: string): PhantomConfig { const configPath = path ?? DEFAULT_CONFIG_PATH; let text: string; @@ -98,6 +142,65 @@ export function loadConfig(path?: string): PhantomConfig { return config; } +/** + * Async loader. Calls `loadConfigSync` and, when `secret_source === "metadata"`, + * resolves the provider token from the host metadata gateway, populates + * `process.env.ANTHROPIC_API_KEY` / `ANTHROPIC_AUTH_TOKEN` (so the unchanged + * `buildProviderEnv` finds it), and walks the parsed config replacing any + * `${secret:NAME}` references with their resolved plaintext. + * + * For `secret_source === "env"` (the default) this is a sync-fast path that + * just returns the parsed config. + */ +export async function loadConfig(path?: string): Promise { + const config = loadConfigSync(path); + + if (config.secret_source !== "metadata") { + return config; + } + + const baseUrl = config.secret_source_url ?? DEFAULT_METADATA_BASE_URL; + const fetcher = new MetadataSecretFetcher(baseUrl); + + // Resolve the provider token first so process.env is populated before any + // downstream code that reads it. Both ANTHROPIC_API_KEY and ANTHROPIC_AUTH_TOKEN + // are set, mirroring the dual-header pattern in buildProviderEnv: the bundled + // Agent SDK auth factory prefers ANTHROPIC_API_KEY, but third-party proxies + // sometimes accept only ANTHROPIC_AUTH_TOKEN. + const providerToken = await fetcher.get(config.provider.secret_name); + process.env.ANTHROPIC_API_KEY = providerToken; + process.env.ANTHROPIC_AUTH_TOKEN = providerToken; + + await interpolateSecretsInPlace(config as unknown as Record, fetcher); + + return config; +} + +/** + * Recursively walk an object tree. For every string-typed leaf whose value + * matches `${secret:NAME}` as the WHOLE string, replace it with the resolved + * plaintext. Plain nested objects recurse; arrays and other non-plain values + * are skipped (no schema field is array-of-strings today, and adding array + * support speculatively would expand the security surface). + * + * Mutates `obj` in place; returns `Promise`. + */ +async function interpolateSecretsInPlace(obj: Record, fetcher: MetadataSecretFetcher): Promise { + for (const [key, value] of Object.entries(obj)) { + if (typeof value === "string") { + const match = value.match(SECRET_REF_REGEX); + if (match) { + const name = match[1]; + if (name) { + obj[key] = await fetcher.get(name); + } + } + } else if (value && typeof value === "object" && !Array.isArray(value)) { + await interpolateSecretsInPlace(value as Record, fetcher); + } + } +} + /** * Load channel configurations with environment variable substitution. * Returns null if the config file doesn't exist (channels are optional). diff --git a/src/config/metadata-fetcher.ts b/src/config/metadata-fetcher.ts new file mode 100644 index 00000000..7e7ebc41 --- /dev/null +++ b/src/config/metadata-fetcher.ts @@ -0,0 +1,83 @@ +// Phase C: tenant Phantom fetches its provider API key from the host metadata +// gateway at boot time instead of reading it from process.env. This file is +// the HTTP client that speaks to http://169.254.169.254/v1/secrets/. +// +// Two security invariants live here: +// 1. Plaintext NEVER appears in error messages, log lines, or thrown errors. +// Errors carry the secret NAME and HTTP status, never the body. +// 2. Secret name is validated against a strict regex BEFORE the fetch fires. +// Anything outside [a-z_][a-z0-9_]* is rejected with a clear error so a +// future caller cannot smuggle path components or query strings into the +// gateway URL. +// +// Caching: a 60s in-process TTL avoids re-fetching on every Agent SDK +// subprocess spawn. ETag (X-Phantom-Rotation-Id) lets the gateway answer 304 +// when the secret has not rotated, refreshing TTL without re-disclosing +// plaintext. Per the brief, no global singleton: loadConfig constructs one +// fetcher per call. + +export const METADATA_CACHE_TTL_MS = 60_000; + +const VALID_SECRET_NAME = /^[a-z_][a-z0-9_]*$/; + +type CacheEntry = { + value: string; + rotationId: string; + fetchedAt: number; +}; + +export class MetadataSecretFetcher { + private readonly baseUrl: string; + private readonly cache = new Map(); + + constructor(baseUrl: string) { + this.baseUrl = baseUrl; + } + + async get(name: string): Promise { + // Defense-in-depth name check. The loader walker enforces the same regex, + // but a future caller could invoke get() directly. Reject anything that + // could smuggle a path or query into the gateway URL. + if (!VALID_SECRET_NAME.test(name)) { + throw new Error(`metadata: invalid secret name: ${name}`); + } + + const cached = this.cache.get(name); + if (cached && Date.now() - cached.fetchedAt < METADATA_CACHE_TTL_MS) { + return cached.value; + } + + const url = `${this.baseUrl}/v1/secrets/${encodeURIComponent(name)}`; + const headers: Record = {}; + if (cached) { + headers["If-None-Match"] = `"${cached.rotationId}"`; + } + + let res: Response; + try { + res = await fetch(url, { method: "GET", headers }); + } catch (err) { + const msg = err instanceof Error ? err.message : String(err); + throw new Error(`metadata: fetch ${name} failed: ${msg}`); + } + + if (res.status === 304) { + if (!cached) { + // 304 with no cache is a server bug. Surface it loudly so the operator + // can investigate; never silently treat this as success. + throw new Error(`metadata: fetch ${name} failed: HTTP 304 with no cached entry`); + } + cached.fetchedAt = Date.now(); + return cached.value; + } + + if (res.status !== 200) { + throw new Error(`metadata: fetch ${name} failed: HTTP ${res.status} ${res.statusText}`); + } + + const value = await res.text(); + const rotationId = res.headers.get("X-Phantom-Rotation-Id") ?? "0"; + this.cache.set(name, { value, rotationId, fetchedAt: Date.now() }); + return value; + } +} diff --git a/src/config/providers.ts b/src/config/providers.ts index ff805b7d..eb96a893 100644 --- a/src/config/providers.ts +++ b/src/config/providers.ts @@ -6,6 +6,14 @@ import type { PhantomConfig } from "./types.ts"; // The Agent SDK already understands every knob we need (ANTHROPIC_BASE_URL, // ANTHROPIC_AUTH_TOKEN, ANTHROPIC_DEFAULT_*_MODEL, CLAUDE_CODE_DISABLE_EXPERIMENTAL_BETAS, // API_TIMEOUT_MS). Phantom's job is to expose those knobs through YAML. Nothing more. +// +// Phase C metadata path: when the top-level `secret_source: "metadata"` is set, +// the loader fetches the secret named by `provider.secret_name` from the host +// metadata gateway and pre-populates `process.env.ANTHROPIC_API_KEY` and +// `process.env.ANTHROPIC_AUTH_TOKEN` BEFORE buildProviderEnv runs. This keeps +// the function below unchanged: it continues to read the resolved value via +// the standard env-var path. Cloud tenants and self-host installs share the +// same code path here. export const PROVIDER_TYPES = ["anthropic", "zai", "openrouter", "vllm", "ollama", "litellm", "custom"] as const; @@ -16,6 +24,22 @@ export const ProviderSchema = z type: z.enum(PROVIDER_TYPES).default("anthropic"), base_url: z.string().url().optional(), api_key_env: z.string().min(1).optional(), + // Phase C: when the top-level `secret_source: "metadata"` is set, the + // loader passes this name to the metadata gateway and the resolved + // plaintext is injected into process.env.ANTHROPIC_API_KEY / + // ANTHROPIC_AUTH_TOKEN before buildProviderEnv runs. Default + // "provider_token" so a Cloud tenant who flips secret_source to + // metadata gets the right behavior with no further config. The regex + // matches the metadata fetcher's defense-in-depth check; pinning it at + // the schema level surfaces invalid names at parse time rather than + // crashing at boot inside the fetcher. + secret_name: z + .string() + .regex(/^[a-z_][a-z0-9_]*$/, { + message: + "secret_name must match /^[a-z_][a-z0-9_]*$/ (lowercase letters, digits, underscores; cannot start with a digit)", + }) + .default("provider_token"), model_mappings: z .object({ opus: z.string().min(1).optional(), diff --git a/src/config/schemas.ts b/src/config/schemas.ts index 84ca2fee..8eedcb36 100644 --- a/src/config/schemas.ts +++ b/src/config/schemas.ts @@ -66,6 +66,17 @@ export const PhantomConfigSchema = z.object({ // The effective env vars are computed by buildProviderEnv() in config/providers.ts and // merged into the Agent SDK subprocess environment at query() time. provider: ProviderSchema, + // Phase C: where to source secret values from. "env" (default, existing + // behavior) reads ANTHROPIC_API_KEY from process.env, populated either by + // .env files (Bun auto-loads) or the systemd EnvironmentFile. "metadata" + // fetches from the host metadata gateway at + // http://169.254.169.254/v1/secrets/. Cloud tenants use "metadata"; + // self-host installs continue to use "env" with no config change. + secret_source: z.enum(["env", "metadata"]).default("env"), + // Optional override of the metadata gateway base URL. Defaults to + // http://169.254.169.254 (the link-local address phantomd binds in Phase C). + // Useful for integration tests that point at a fake gateway on localhost. + secret_source_url: z.string().url().optional(), effort: z.enum(["low", "medium", "high", "max"]).default("max"), max_budget_usd: z.number().min(0).default(0), timeout_minutes: z.number().min(1).default(240), diff --git a/src/index.ts b/src/index.ts index 50aed7a6..e68d91a9 100644 --- a/src/index.ts +++ b/src/index.ts @@ -76,7 +76,7 @@ async function main(): Promise { console.log("[phantom] Starting..."); - const config = loadConfig(); + const config = await loadConfig(); console.log(`[phantom] Config loaded: ${config.name} (${config.model}, effort: ${config.effort})`); // Set web UI public directory diff --git a/src/mcp/__tests__/dynamic-tools.test.ts b/src/mcp/__tests__/dynamic-tools.test.ts index 846d4c28..71c38fee 100644 --- a/src/mcp/__tests__/dynamic-tools.test.ts +++ b/src/mcp/__tests__/dynamic-tools.test.ts @@ -269,7 +269,8 @@ describe("Dynamic Tools via MCP Protocol", () => { port: 3100, role: "swe", model: "claude-opus-4-6", - provider: { type: "anthropic" as const }, + provider: { type: "anthropic" as const, secret_name: "provider_token" }, + secret_source: "env" as const, effort: "max" as const, max_budget_usd: 0, timeout_minutes: 240, diff --git a/src/mcp/__tests__/scope-enforcement.test.ts b/src/mcp/__tests__/scope-enforcement.test.ts index fda54515..1e5983a4 100644 --- a/src/mcp/__tests__/scope-enforcement.test.ts +++ b/src/mcp/__tests__/scope-enforcement.test.ts @@ -130,7 +130,8 @@ describe("MCP scope enforcement", () => { port: 3100, role: "swe", model: "claude-opus-4-6", - provider: { type: "anthropic" as const }, + provider: { type: "anthropic" as const, secret_name: "provider_token" }, + secret_source: "env" as const, effort: "max" as const, max_budget_usd: 0, timeout_minutes: 240, diff --git a/src/mcp/__tests__/server.test.ts b/src/mcp/__tests__/server.test.ts index f82bda34..ac3f683d 100644 --- a/src/mcp/__tests__/server.test.ts +++ b/src/mcp/__tests__/server.test.ts @@ -115,7 +115,8 @@ describe("PhantomMcpServer", () => { port: 3100, role: "swe", model: "claude-opus-4-6", - provider: { type: "anthropic" as const }, + provider: { type: "anthropic" as const, secret_name: "provider_token" }, + secret_source: "env" as const, effort: "max" as const, max_budget_usd: 0, timeout_minutes: 240, @@ -333,7 +334,8 @@ describe("PhantomMcpServer", () => { port: 3100, role: "swe", model: "claude-opus-4-6", - provider: { type: "anthropic" as const }, + provider: { type: "anthropic" as const, secret_name: "provider_token" }, + secret_source: "env" as const, effort: "max" as const, max_budget_usd: 0, timeout_minutes: 240, diff --git a/src/mcp/__tests__/tools-swe.test.ts b/src/mcp/__tests__/tools-swe.test.ts index 45e5fb10..2a202857 100644 --- a/src/mcp/__tests__/tools-swe.test.ts +++ b/src/mcp/__tests__/tools-swe.test.ts @@ -130,7 +130,8 @@ describe("SWE MCP Tools", () => { port: 3100, role: "swe", model: "claude-opus-4-6", - provider: { type: "anthropic" as const }, + provider: { type: "anthropic" as const, secret_name: "provider_token" }, + secret_source: "env" as const, effort: "max" as const, max_budget_usd: 0, timeout_minutes: 240,