diff --git a/packages/agent/src/signed-commit-artefacts.test.ts b/packages/agent/src/signed-commit-artefacts.test.ts index 1e33564942..e05b264153 100644 --- a/packages/agent/src/signed-commit-artefacts.test.ts +++ b/packages/agent/src/signed-commit-artefacts.test.ts @@ -1,9 +1,38 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import path from "node:path"; import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, +} from "vitest"; +import { + parseSandboxEnv, reportCommitArtefacts, reportTaskRunBranch, + resolveSandboxPosthogApi, } from "./signed-commit-artefacts"; +describe("parseSandboxEnv", () => { + it("parses NUL-delimited entries into an object", () => { + expect(parseSandboxEnv("FIRST=one\0SECOND=two\0")).toEqual({ + FIRST: "one", + SECOND: "two", + }); + }); + + it("preserves equals signs in values and ignores malformed entries", () => { + expect( + parseSandboxEnv("TOKEN=header.payload=signature\0invalid\0=value\0"), + ).toEqual({ TOKEN: "header.payload=signature" }); + }); +}); + const ENV = { POSTHOG_API_URL: "https://us.posthog.com", POSTHOG_PERSONAL_API_KEY: "pha_test", @@ -12,6 +41,97 @@ const ENV = { // Point the env-file read at a path that never exists so only `env` is used. const NO_ENV_FILE = "/nonexistent/agent-env"; +const TEST_OAUTH_ENV_FILE = path.join( + tmpdir(), + `posthog-agent-oauth-env-${process.pid}`, +); + +beforeAll(async () => { + await writeFile(TEST_OAUTH_ENV_FILE, "POSTHOG_PERSONAL_API_KEY=pha_test\0"); +}); + +afterAll(async () => { + await rm(TEST_OAUTH_ENV_FILE, { force: true }); +}); + +describe("resolveSandboxPosthogApi", () => { + it("reads the rotating API key from the dedicated OAuth file", async () => { + const directory = await mkdtemp( + path.join(tmpdir(), "sandbox-posthog-api-"), + ); + const envFilePath = path.join(directory, "agent-env"); + const oauthEnvFilePath = path.join(directory, "agent-oauth-env"); + + try { + await writeFile( + envFilePath, + "POSTHOG_API_URL=https://us.posthog.com\0POSTHOG_PROJECT_ID=7\0", + ); + await writeFile( + oauthEnvFilePath, + "POSTHOG_PERSONAL_API_KEY=pha_refreshed\0", + ); + + expect( + resolveSandboxPosthogApi({}, envFilePath, oauthEnvFilePath), + ).toEqual({ + apiUrl: "https://us.posthog.com", + apiKey: "pha_refreshed", + projectId: 7, + }); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + it("fails closed without the dedicated OAuth file", () => { + expect( + resolveSandboxPosthogApi(ENV, NO_ENV_FILE, NO_ENV_FILE), + ).toBeUndefined(); + }); + + it("does not resurrect a stale token when the OAuth file is empty", async () => { + const directory = await mkdtemp( + path.join(tmpdir(), "sandbox-posthog-api-"), + ); + const envFilePath = path.join(directory, "agent-env"); + const oauthEnvFilePath = path.join(directory, "agent-oauth-env"); + + try { + await writeFile( + envFilePath, + "POSTHOG_API_URL=https://us.posthog.com\0POSTHOG_PERSONAL_API_KEY=pha_stale\0POSTHOG_PROJECT_ID=7\0", + ); + await writeFile(oauthEnvFilePath, ""); + + expect( + resolveSandboxPosthogApi(ENV, envFilePath, oauthEnvFilePath), + ).toBeUndefined(); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + it("fails closed when the managed OAuth file is unreadable", async () => { + const directory = await mkdtemp( + path.join(tmpdir(), "sandbox-posthog-api-"), + ); + const envFilePath = path.join(directory, "agent-env"); + + try { + await writeFile( + envFilePath, + "POSTHOG_API_URL=https://us.posthog.com\0POSTHOG_PROJECT_ID=7\0", + ); + + expect( + resolveSandboxPosthogApi(ENV, envFilePath, directory), + ).toBeUndefined(); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); +}); const RESULT = { branch: "posthog-code/fix-foo", @@ -59,6 +179,7 @@ describe("reportCommitArtefacts", () => { message: "fix: foo", env: ENV, envFilePath: NO_ENV_FILE, + oauthEnvFilePath: TEST_OAUTH_ENV_FILE, }); const lookupCalls = fetchMock.mock.calls.filter(([url]) => @@ -86,6 +207,83 @@ describe("reportCommitArtefacts", () => { } }); + it("rereads the OAuth file when credentials rotate between requests", async () => { + const directory = await mkdtemp( + path.join(tmpdir(), "sandbox-posthog-api-"), + ); + const oauthEnvFilePath = path.join(directory, "agent-oauth-env"); + await writeFile(oauthEnvFilePath, "POSTHOG_PERSONAL_API_KEY=pha_initial\0"); + + try { + fetchMock.mockImplementationOnce(async () => { + await writeFile( + oauthEnvFilePath, + "POSTHOG_PERSONAL_API_KEY=pha_rotated\0", + ); + return jsonResponse({ results: [{ id: "report-1" }] }); + }); + fetchMock.mockImplementation(async () => + jsonResponse({ id: "artefact" }), + ); + + await reportCommitArtefacts({ + taskId: "task-1", + result: { ...RESULT, commits: [RESULT.commits[0]] }, + message: "fix: foo", + env: ENV, + envFilePath: NO_ENV_FILE, + oauthEnvFilePath, + }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect( + (fetchMock.mock.calls[0]?.[1]?.headers as Headers).get("Authorization"), + ).toBe("Bearer pha_initial"); + expect( + (fetchMock.mock.calls[1]?.[1]?.headers as Headers).get("Authorization"), + ).toBe("Bearer pha_rotated"); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + + it("rereads the OAuth file when retrying an auth failure", async () => { + const directory = await mkdtemp( + path.join(tmpdir(), "sandbox-posthog-api-"), + ); + const oauthEnvFilePath = path.join(directory, "agent-oauth-env"); + await writeFile(oauthEnvFilePath, "POSTHOG_PERSONAL_API_KEY=pha_initial\0"); + + try { + fetchMock.mockImplementationOnce(async () => { + await writeFile( + oauthEnvFilePath, + "POSTHOG_PERSONAL_API_KEY=pha_rotated\0", + ); + return new Response(null, { status: 401 }); + }); + fetchMock.mockImplementationOnce(async () => + jsonResponse({ results: [] }), + ); + + await reportCommitArtefacts({ + taskId: "task-1", + result: RESULT, + message: "fix: foo", + env: ENV, + envFilePath: NO_ENV_FILE, + oauthEnvFilePath, + }); + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect( + (fetchMock.mock.calls[1]?.[1]?.headers as Headers).get("Authorization"), + ).toBe("Bearer pha_rotated"); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }); + it("is a no-op without a task id", async () => { await reportCommitArtefacts({ taskId: undefined, @@ -117,6 +315,7 @@ describe("reportCommitArtefacts", () => { message: "fix: foo", env: ENV, envFilePath: NO_ENV_FILE, + oauthEnvFilePath: TEST_OAUTH_ENV_FILE, }), ).resolves.toBeUndefined(); }); @@ -140,6 +339,7 @@ describe("reportCommitArtefacts", () => { message: "fix: foo", env: ENV, envFilePath: NO_ENV_FILE, + oauthEnvFilePath: TEST_OAUTH_ENV_FILE, }); // Both commits attempted despite the first failing. @@ -175,6 +375,7 @@ describe("reportTaskRunBranch", () => { branch: "posthog-code/fix-foo", env: ENV, envFilePath: NO_ENV_FILE, + oauthEnvFilePath: TEST_OAUTH_ENV_FILE, }); expect(fetchMock).toHaveBeenCalledOnce(); diff --git a/packages/agent/src/signed-commit-artefacts.ts b/packages/agent/src/signed-commit-artefacts.ts index 0d32f4bfdf..d5690d6948 100644 --- a/packages/agent/src/signed-commit-artefacts.ts +++ b/packages/agent/src/signed-commit-artefacts.ts @@ -3,6 +3,7 @@ import type { SignedCommitResult } from "@posthog/git/signed-commit"; import { PostHogAPIClient } from "./posthog-api"; const SANDBOX_ENV_FILE = "/tmp/agent-env"; +const SANDBOX_OAUTH_ENV_FILE = "/tmp/agent-oauth-env"; /** * Best-effort "commit hook": after a successful signed-commit push, record one `commit` @@ -11,11 +12,10 @@ const SANDBOX_ENV_FILE = "/tmp/agent-env"; * endpoint reads the `X-PostHog-Task-Id` header, never the model. * * Credentials come from the sandbox environment (`POSTHOG_API_URL` / - * `POSTHOG_PERSONAL_API_KEY` / `POSTHOG_PROJECT_ID`), preferring the live agentsh env file - * for the key so a mid-session token refresh is picked up — the same pattern as - * `resolveGithubToken`. Works identically from the Claude in-process server and the Codex - * stdio child (both inherit the sandbox env). Never throws: a failed artefact post must not - * fail the commit that already landed. + * `POSTHOG_PERSONAL_API_KEY` / `POSTHOG_PROJECT_ID`), preferring the dedicated live agentsh + * OAuth file for the key so a mid-session token refresh is picked up — the same pattern as + * `resolveGithubToken`. Never throws: a failed artefact post must not fail the commit that + * already landed. */ interface SandboxPosthogApi { @@ -24,52 +24,70 @@ interface SandboxPosthogApi { projectId: number; } +export function parseSandboxEnv(raw: string): Record { + const env: Record = {}; + for (const entry of raw.split("\0")) { + const separator = entry.indexOf("="); + if (separator > 0) { + env[entry.slice(0, separator)] = entry.slice(separator + 1); + } + } + return env; +} + function readSandboxEnvFile(envFilePath: string): Record { try { - const raw = readFileSync(envFilePath, "utf8"); - const env: Record = {}; - for (const entry of raw.split("\0")) { - const eq = entry.indexOf("="); - if (eq > 0) { - env[entry.slice(0, eq)] = entry.slice(eq + 1); - } - } - return env; + return parseSandboxEnv(readFileSync(envFilePath, "utf8")); } catch { // No env file (local/desktop or test) — fall back to the process env only. return {}; } } +function readSandboxOauthToken(oauthEnvFilePath: string): string | undefined { + try { + const raw = readFileSync(oauthEnvFilePath, "utf8"); + return parseSandboxEnv(raw).POSTHOG_PERSONAL_API_KEY ?? ""; + } catch { + // The dedicated credential channel is mandatory. Missing and unreadable + // files both fail closed. + return undefined; + } +} + export function resolveSandboxPosthogApi( env: Record = process.env, envFilePath: string = SANDBOX_ENV_FILE, + oauthEnvFilePath: string = SANDBOX_OAUTH_ENV_FILE, ): SandboxPosthogApi | undefined { const fileEnv = readSandboxEnvFile(envFilePath); + const oauthToken = readSandboxOauthToken(oauthEnvFilePath); const apiUrl = fileEnv.POSTHOG_API_URL ?? env.POSTHOG_API_URL; - const apiKey = - fileEnv.POSTHOG_PERSONAL_API_KEY ?? env.POSTHOG_PERSONAL_API_KEY; const projectId = Number( fileEnv.POSTHOG_PROJECT_ID ?? env.POSTHOG_PROJECT_ID, ); - if (!apiUrl || !apiKey || !Number.isFinite(projectId) || projectId <= 0) { + if (!apiUrl || !oauthToken || !Number.isFinite(projectId) || projectId <= 0) { return undefined; } - return { apiUrl, apiKey, projectId }; + return { apiUrl, apiKey: oauthToken, projectId }; } export function createSandboxPosthogClient( env?: Record, envFilePath?: string, + oauthEnvFilePath?: string, ): PostHogAPIClient | undefined { - const api = resolveSandboxPosthogApi(env, envFilePath); + const api = resolveSandboxPosthogApi(env, envFilePath, oauthEnvFilePath); if (!api) { return undefined; } return new PostHogAPIClient({ apiUrl: api.apiUrl, projectId: api.projectId, - getApiKey: () => api.apiKey, + getApiKey: () => + readSandboxOauthToken(oauthEnvFilePath ?? SANDBOX_OAUTH_ENV_FILE) ?? "", + refreshApiKey: () => + readSandboxOauthToken(oauthEnvFilePath ?? SANDBOX_OAUTH_ENV_FILE) ?? "", }); } @@ -80,13 +98,18 @@ export async function reportCommitArtefacts(opts: { message: string; env?: Record; envFilePath?: string; + oauthEnvFilePath?: string; }): Promise { const { taskId, result, message } = opts; if (!taskId) { return; // Local/desktop run — no task to attribute or associate through. } try { - const client = createSandboxPosthogClient(opts.env, opts.envFilePath); + const client = createSandboxPosthogClient( + opts.env, + opts.envFilePath, + opts.oauthEnvFilePath, + ); if (!client) { return; // No sandbox PostHog credentials — nothing to report to. } @@ -121,12 +144,17 @@ export async function reportTaskRunBranch(opts: { branch: string; env?: Record; envFilePath?: string; + oauthEnvFilePath?: string; }): Promise { if (!opts.taskId || !opts.taskRunId) { return; } try { - const client = createSandboxPosthogClient(opts.env, opts.envFilePath); + const client = createSandboxPosthogClient( + opts.env, + opts.envFilePath, + opts.oauthEnvFilePath, + ); if (!client) { return; }