diff --git a/.changeset/run-agent-secrets-filter.md b/.changeset/run-agent-secrets-filter.md new file mode 100644 index 0000000000..ba31dcda8b --- /dev/null +++ b/.changeset/run-agent-secrets-filter.md @@ -0,0 +1,6 @@ +--- +'@expo/build-tools': none +'@expo/worker': none +--- + +Mask the credentials `eas/run_agent` leases with the build log secrets filter instead of redacting them in the step. diff --git a/packages/build-tools/src/context.ts b/packages/build-tools/src/context.ts index 35f7393e59..6955ad4ca4 100644 --- a/packages/build-tools/src/context.ts +++ b/packages/build-tools/src/context.ts @@ -69,6 +69,7 @@ export interface BuildContextOptions { metadata?: Metadata; expoApiV2BaseUrl?: string; mcpServerUrl?: string; + addBuildLogMask?: (secret: string) => void; } export class SkipNativeBuildError extends Error {} @@ -86,6 +87,7 @@ export class BuildContext { public readonly skipNativeBuild?: boolean; public readonly expoApiV2BaseUrl?: string; public readonly mcpServerUrl?: string; + public readonly addBuildLogMask?: (secret: string) => void; public artifacts: Artifacts = {}; private readonly _isLocal: boolean; @@ -115,6 +117,7 @@ export class BuildContext { this.skipNativeBuild = options.skipNativeBuild; this.expoApiV2BaseUrl = options.expoApiV2BaseUrl; this.mcpServerUrl = options.mcpServerUrl; + this.addBuildLogMask = options.addBuildLogMask; const environmentSecrets = this.getEnvironmentSecrets(job); this._env = { diff --git a/packages/build-tools/src/customBuildContext.ts b/packages/build-tools/src/customBuildContext.ts index 148faf6498..022524cbe2 100644 --- a/packages/build-tools/src/customBuildContext.ts +++ b/packages/build-tools/src/customBuildContext.ts @@ -32,6 +32,7 @@ export interface BuilderRuntimeApi { uploadArtifact: (spec: { artifact: ArtifactToUpload; logger: bunyan }) => Promise<{ artifactId: string | null; }>; + addBuildLogMask?: (secret: string) => void; } export class CustomBuildContext implements ExternalBuildContextProvider { @@ -84,6 +85,7 @@ export class CustomBuildContext implements ExternalBuild this.buildLogsDirectory = path.join(buildCtx.workingdir, 'logs'); this.runtimeApi = { uploadArtifact: (...args) => buildCtx['uploadArtifact'](...args), + addBuildLogMask: buildCtx.addBuildLogMask, }; this.mcpServerUrl = buildCtx.mcpServerUrl; this.expoApiV2BaseUrl = buildCtx.expoApiV2BaseUrl; diff --git a/packages/build-tools/src/steps/functions/__tests__/runAgent.test.ts b/packages/build-tools/src/steps/functions/__tests__/runAgent.test.ts index d90f132183..763e28c38a 100644 --- a/packages/build-tools/src/steps/functions/__tests__/runAgent.test.ts +++ b/packages/build-tools/src/steps/functions/__tests__/runAgent.test.ts @@ -25,6 +25,7 @@ const leaseMock = jest.mocked(leaseAgentRunProviderCredentialsAsync); const ctx = { expoApiV2BaseUrl: 'https://staging-api.expo.test/v2/', mcpServerUrl: 'wss://staging-mcp.expo.test', + runtimeApi: { addBuildLogMask: jest.fn() }, } as any; const inputs = { agent_kind: { value: 'claude-code' }, @@ -115,6 +116,19 @@ describe(createRunAgentBuildFunction, () => { expect(agentEnv.HOME).not.toBe('/home/expo'); }); + it('hands the build log secrets filter to the agent run', async () => { + const runAgentSpy = jest.spyOn(agentRun, 'runAgentAsync').mockResolvedValue(); + try { + await runAsync(); + + expect(runAgentSpy).toHaveBeenCalledWith( + expect.objectContaining({ addBuildLogMask: ctx.runtimeApi.addBuildLogMask }) + ); + } finally { + runAgentSpy.mockRestore(); + } + }); + it.each([ ['wss://staging-mcp.expo.test', 'https://staging-mcp.expo.test/mcp'], ['ws://localhost:8787', 'http://localhost:8787/mcp'], @@ -162,6 +176,7 @@ describe(createRunAgentBuildFunction, () => { ['no PATH', { env: { ...env, PATH: '' } }], ['no Expo API URL', { ctx: { ...ctx, expoApiV2BaseUrl: undefined } }], ['no MCP server URL', { ctx: { ...ctx, mcpServerUrl: undefined } }], + ['no build log secrets filter', { ctx: { ...ctx, runtimeApi: {} } }], ])('throws a system error for %s', async (_name, overrides) => { await expect(runAsync(overrides)).rejects.toBeInstanceOf(SystemError); diff --git a/packages/build-tools/src/steps/functions/runAgent.ts b/packages/build-tools/src/steps/functions/runAgent.ts index f6a5c51477..57667b6992 100644 --- a/packages/build-tools/src/steps/functions/runAgent.ts +++ b/packages/build-tools/src/steps/functions/runAgent.ts @@ -96,6 +96,9 @@ export function createRunAgentBuildFunction(ctx: CustomBuildContext): BuildFunct if (!ctx.mcpServerUrl) { throw new SystemError('MCP server URL is required to run an agent.'); } + if (!ctx.runtimeApi.addBuildLogMask) { + throw new SystemError('A build log secrets filter is required to run an agent.'); + } // The worker config holds the MCP server's WebSocket URL; its HTTP endpoint is on the same host. const mcpUrl = new URL('/mcp', ctx.mcpServerUrl); mcpUrl.protocol = mcpUrl.protocol.replace(/^ws/, 'http'); @@ -120,6 +123,7 @@ export function createRunAgentBuildFunction(ctx: CustomBuildContext): BuildFunct pathEnv, npmCacheUrl, logger: stepCtx.logger, + addBuildLogMask: ctx.runtimeApi.addBuildLogMask, signal: signal ? AbortSignal.any([signal, timeoutController.signal]) : timeoutController.signal, diff --git a/packages/build-tools/src/steps/utils/__tests__/agentRun.test.ts b/packages/build-tools/src/steps/utils/__tests__/agentRun.test.ts index 24b6e6b2c6..b229ab374b 100644 --- a/packages/build-tools/src/steps/utils/__tests__/agentRun.test.ts +++ b/packages/build-tools/src/steps/utils/__tests__/agentRun.test.ts @@ -140,6 +140,7 @@ function createOptions( pathEnv: '/usr/local/bin:/usr/bin', npmCacheUrl: 'https://npm-cache.expo.test', logger: createMockLogger(), + addBuildLogMask: jest.fn(), signal: new AbortController().signal, ...overrides, }; @@ -220,23 +221,22 @@ describe(runAgentAsync, () => { expect(agent.stdin).toHaveBeenCalledWith('Fix the failing test.'); }); - it('writes the agent output as log lines with the leased credentials masked', async () => { + it('writes the agent output as log lines', async () => { leaseMock.mockResolvedValue(anthropicLease); const logger = createMockLogger(); mockSpawn({ onAgent: agent => { - // One JSON event split across two chunks, in the middle of the token. + const toolResultEvent = + '{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"toolu_1","content":"3 tests passed"}]}}\n'; + agent.child.stdout.write(toolResultEvent.slice(0, 40)); + agent.child.stdout.write(toolResultEvent.slice(40)); agent.child.stdout.write( - '{"type":"user","message":{"content":[{"type":"tool_result","tool_use_id":"toolu_1","content":"token: anthropic-acc' - ); - agent.child.stdout.write('ess-token"}]}}\n'); - agent.child.stdout.write( - '{"type":"assistant","message":{"content":[{"type":"text","text":"Done.\\nThe token was anthropic-access-token.\\n"}]}}\n' + '{"type":"assistant","message":{"content":[{"type":"text","text":"Done.\\nThe tests pass.\\n"}]}}\n' ); agent.child.stdout.write( '{"type":"result","subtype":"error_during_execution","is_error":true,"errors":["boom"]}\n' ); - agent.child.stderr.write('warning: anthropic-access-token\n'); + agent.child.stderr.write('warning: deprecated flag\n'); agent.exit(0); }, }); @@ -247,11 +247,54 @@ describe(runAgentAsync, () => { expect(child.mock.calls).toEqual([[{ source: 'stdout' }], [{ source: 'stderr' }]]); const [stdoutLogger, stderrLogger] = child.mock.results.map(result => result.value); expect(stdoutLogger.info.mock.calls).toEqual([ - ['Tool result: token: [redacted]'], - ['Agent: Done.\nThe token was [redacted].'], + ['Tool result: 3 tests passed'], + ['Agent: Done.\nThe tests pass.'], ]); expect(stdoutLogger.error.mock.calls).toEqual([['Agent failed: boom']]); - expect(stderrLogger.info.mock.calls).toEqual([['warning: [redacted]']]); + expect(stderrLogger.info.mock.calls).toEqual([['warning: deprecated flag']]); + }); + + it.each([ + ['claude-code', anthropicLease, ['anthropic-access-token']], + ['codex', openaiLease, ['openai-access-token', 'openai-id-token']], + ] as const)( + 'registers the credentials leased for %s with the build log secrets filter before starting the agent', + async (agentKind, lease, secrets) => { + leaseMock.mockResolvedValue(lease); + const addBuildLogMask = jest.fn(); + const invocations = mockSpawn({ + onAgent: agent => { + expect(addBuildLogMask.mock.calls).toEqual(secrets.map(secret => [secret])); + agent.exit(0); + }, + }); + + await runAgentAsync(createOptions({ agentKind, addBuildLogMask })); + + expect(invocations).toHaveLength(1); + } + ); + + it('registers renewed credentials before resuming the agent', async () => { + leaseMock + .mockResolvedValueOnce(anthropicLease) + .mockResolvedValueOnce({ ...anthropicLease, accessToken: 'renewed-access-token' }); + const addBuildLogMask = jest.fn(); + const invocations = mockSpawn({ + onAgent: agent => { + if (invocations.length === 2) { + expect(addBuildLogMask.mock.calls).toEqual([ + ['anthropic-access-token'], + ['renewed-access-token'], + ]); + } + agent.exit(invocations.length === 1 ? 1 : 0); + }, + }); + + await runAgentAsync(createOptions({ addBuildLogMask })); + + expect(invocations).toHaveLength(2); }); it('reports a line the formatter cannot handle instead of ending the run', async () => { @@ -427,12 +470,7 @@ describe(runAgentAsync, () => { leaseMock.mockResolvedValueOnce(anthropicLease).mockResolvedValueOnce(renewedLease); const logger = createMockLogger(); const invocations = mockSpawn({ - onAgent: agent => { - if (invocations.length === 2) { - agent.child.stderr.write('tokens: anthropic-access-token renewed-access-token\n'); - } - agent.exit(invocations.length === 1 ? 1 : 0); - }, + onAgent: agent => agent.exit(invocations.length === 1 ? 1 : 0), }); await runAgentAsync(createOptions({ logger })); @@ -452,8 +490,6 @@ describe(runAgentAsync, () => { [{ source: 'stdout' }], [{ source: 'stderr' }], ]); - const [, , , resumedStderrLogger] = child.mock.results.map(result => result.value); - expect(resumedStderrLogger.info.mock.calls).toEqual([['tokens: [redacted] [redacted]']]); }); it('resumes Codex with new credentials when the lease changed during the run', async () => { diff --git a/packages/build-tools/src/steps/utils/agentRun.ts b/packages/build-tools/src/steps/utils/agentRun.ts index e714872c59..805c0bb03c 100644 --- a/packages/build-tools/src/steps/utils/agentRun.ts +++ b/packages/build-tools/src/steps/utils/agentRun.ts @@ -35,6 +35,7 @@ export async function runAgentAsync({ pathEnv, npmCacheUrl, logger, + addBuildLogMask, signal, }: { agentKind: AgentKind; @@ -47,11 +48,11 @@ export async function runAgentAsync({ pathEnv: string; npmCacheUrl?: string; logger: bunyan; + addBuildLogMask: (secret: string) => void; signal: AbortSignal; }): Promise { const leaseOptions = { expoApiV2BaseUrl, expoToken, agentRunId, signal }; let credentials = await leaseAgentRunProviderCredentialsAsync(leaseOptions); - const secrets = new Set(); const runDirectory = await fs.mkdtemp(path.join(os.tmpdir(), 'eas-agent-run-')); try { @@ -126,14 +127,13 @@ export async function runAgentAsync({ } for (const secret of invocation.secrets) { - secrets.add(secret); + addBuildLogMask(secret); } const { exitCode, exitSignal } = await runAgentProcessAsync({ command: path.join(cliDirectory, 'node_modules', '.bin', bin), ...invocation, cwd: workingDirectory, formatOutputLine, - secrets: [...secrets], logger, signal, }); @@ -170,7 +170,6 @@ async function runAgentProcessAsync({ cwd, stdin, formatOutputLine, - secrets, logger, signal, }: { @@ -180,7 +179,6 @@ async function runAgentProcessAsync({ cwd: string; stdin: string; formatOutputLine: AgentCli['formatOutputLine']; - secrets: string[]; logger: bunyan; signal: AbortSignal; }): Promise<{ exitCode: number | null; exitSignal: NodeJS.Signals | null }> { @@ -188,8 +186,6 @@ async function runAgentProcessAsync({ // Detached, so the agent leads a process group that can be signaled as a whole. const child = spawn(command, args, { cwd, env, detached: true }); const outputClosed = new Promise(resolve => child.once('close', () => resolve())); - const redact = (text: string): string => - secrets.reduce((masked, secret) => masked.replaceAll(secret, '[redacted]'), text); const stdoutLogger = logger.child({ source: 'stdout' }); readline.createInterface({ input: child.stdout }).on('line', line => { let messages: AgentLogMessage[]; @@ -201,12 +197,12 @@ async function runAgentProcessAsync({ return; } for (const { level, message } of messages) { - stdoutLogger[level](redact(message).trimEnd()); + stdoutLogger[level](message.trimEnd()); } }); const stderrLogger = logger.child({ source: 'stderr' }); readline.createInterface({ input: child.stderr }).on('line', line => { - stderrLogger.info(redact(line)); + stderrLogger.info(line); }); // The agent may exit before it reads the prompt; its exit status reports that. child.stdin.on('error', () => {}); diff --git a/packages/worker/src/__unit__/context.test.ts b/packages/worker/src/__unit__/context.test.ts index b2ea62e0f6..d7ec60b542 100644 --- a/packages/worker/src/__unit__/context.test.ts +++ b/packages/worker/src/__unit__/context.test.ts @@ -38,6 +38,7 @@ describe(createBuildContext.name, () => { projectId: 'project-id', buildId: 'build-id', buildLogger, + addBuildLogMask: jest.fn(), }; beforeEach(() => { diff --git a/packages/worker/src/__unit__/logger.test.ts b/packages/worker/src/__unit__/logger.test.ts index 9c9e14759d..ad31acf8cc 100644 --- a/packages/worker/src/__unit__/logger.test.ts +++ b/packages/worker/src/__unit__/logger.test.ts @@ -135,6 +135,56 @@ describe('logger', () => { expect(logs[1].msg).toBe('another ****** in base64 is ********************'); }); + it('obfuscates secrets added after the logger was created', async () => { + const { logger, outputStream, logBuffer, addBuildLogMask } = + await createBuildLoggerWithSecretsFilter({}); + + const logs: any[] = []; + outputStream.pipe( + new Writable({ + objectMode: true, + write(chunk: any, _encoding: BufferEncoding, callback: TransformCallback) { + logs.push(chunk); + callback(null, chunk); + }, + }) + ); + + addBuildLogMask('leased-token'); + addBuildLogMask(Buffer.from('decodedToken').toString('base64')); + addBuildLogMask('production'); + logger.child({ source: 'stdout' }).info('leased-token, decodedToken, production'); + + await waitForStreamFlush(); + + expect(logs.map(log => log.msg)).toEqual(['************, ************, production']); + expect(logBuffer.getLogs()).toEqual(['************, ************, production']); + }); + + it('obfuscates a secret that contains another secret', async () => { + const { logger, outputStream, addBuildLogMask } = await createBuildLoggerWithSecretsFilter({ + environmentSecrets: [{ name: 'PREFIX', value: 'sk-', type: EnvironmentSecretType.STRING }], + }); + + const logs: any[] = []; + outputStream.pipe( + new Writable({ + objectMode: true, + write(chunk: any, _encoding: BufferEncoding, callback: TransformCallback) { + logs.push(chunk); + callback(null, chunk); + }, + }) + ); + + addBuildLogMask('sk-leased-token'); + logger.info('token: sk-leased-token, prefix: sk-'); + + await waitForStreamFlush(); + + expect(logs.map(log => log.msg)).toEqual(['token: ***************, prefix: ***']); + }); + it('adds logId to each log', async () => { const { logger, outputStream } = await createBuildLoggerWithSecretsFilter({}); diff --git a/packages/worker/src/__unit__/service.test.ts b/packages/worker/src/__unit__/service.test.ts index 8560f40b9d..45573c54a3 100644 --- a/packages/worker/src/__unit__/service.test.ts +++ b/packages/worker/src/__unit__/service.test.ts @@ -114,6 +114,7 @@ describe('BuildService Datadog setup', () => { createBuildContextMock.mockResolvedValue({ job: {} } as any); createBuildLoggerWithSecretsFilterMock.mockResolvedValue({ logger: buildLogger, + addBuildLogMask: jest.fn(), cleanUp: jest.fn(async () => {}), logBuffer: { getLogs: () => [], getPhaseLogs: () => [] } as any, outputStream: {} as any, diff --git a/packages/worker/src/context.ts b/packages/worker/src/context.ts index e8ff68a7a8..ffab76b6af 100644 --- a/packages/worker/src/context.ts +++ b/packages/worker/src/context.ts @@ -26,6 +26,7 @@ export async function createBuildContext({ projectId, buildId, buildLogger, + addBuildLogMask, }: { job: TJob; logBuffer: LogBuffer; @@ -34,6 +35,7 @@ export async function createBuildContext({ projectId: string; buildId: string; buildLogger: bunyan; + addBuildLogMask: (secret: string) => void; }): Promise> { const childLogger = buildLogger.child({ buildId }); await RuntimeSettings.loadAsync({ @@ -104,6 +106,7 @@ export async function createBuildContext({ metadata, expoApiV2BaseUrl: config.wwwApiV2BaseUrl, mcpServerUrl: config.mcpServerUrl, + addBuildLogMask, }); return ctx; } diff --git a/packages/worker/src/logger.ts b/packages/worker/src/logger.ts index 70b70209be..167ca2f6ab 100644 --- a/packages/worker/src/logger.ts +++ b/packages/worker/src/logger.ts @@ -1,5 +1,5 @@ import { HttpLogStream, LogBuffer, RemoteLoggerStream } from '@expo/build-tools'; -import { BuildPhase, EnvironmentSecret, Job } from '@expo/eas-build-job'; +import { BuildPhase, Job } from '@expo/eas-build-job'; import { LoggerLevel, bunyan, createLogger } from '@expo/logger'; import { Readable, Transform, TransformCallback, Writable } from 'stream'; @@ -23,7 +23,10 @@ export class WorkerLogBuffer extends Writable implements LogBuffer { public writable: boolean = true; public buffer: CachedLog[] = []; - constructor(private readonly numberOfLines: number) { + constructor( + private readonly numberOfLines: number, + private readonly secretPatterns: ReadonlySet + ) { super(); } @@ -43,7 +46,7 @@ export class WorkerLogBuffer extends Writable implements LogBuffer { if (this.buffer.length >= this.numberOfLines) { this.buffer.shift(); } - this.buffer.push({ msg: rec.msg, phase: rec.phase }); + this.buffer.push({ msg: maskSecrets(rec.msg, this.secretPatterns), phase: rec.phase }); return true; } @@ -56,13 +59,7 @@ export class WorkerLogBuffer extends Writable implements LogBuffer { } } -function createTransformStream(secrets: EnvironmentSecret[]): Transform { - const secretValues = secrets.map(({ value }) => value); - const secretList: string[] = [ - ...secretValues, - ...secretValues.map(maybeStringBase64Decode).filter((i): i is string => !!i), - ].filter(i => i.length > 1 && !simpleSecretsWhitelist.includes(i)); - +function createTransformStream(secretPatterns: ReadonlySet): Transform { return new Transform({ readableObjectMode: true, writableObjectMode: true, @@ -79,13 +76,9 @@ function createTransformStream(secrets: EnvironmentSecret[]): Transform { } if (chunk && typeof chunk === 'object' && chunk.msg) { - const msgWithoutSecrets = secretList.reduce((acc: string, pattern: string): string => { - return acc.replaceAll(pattern, '*'.repeat(pattern.length)); - }, chunk.msg as string); - callback(null, { ...chunk, - msg: msgWithoutSecrets, + msg: maskSecrets(chunk.msg as string, secretPatterns), }); } else { callback(null, chunk); @@ -94,6 +87,20 @@ function createTransformStream(secrets: EnvironmentSecret[]): Transform { }); } +function maskSecrets(message: string, secrets: Iterable): string { + const characters = message.split(''); + for (const secret of secrets) { + for ( + let start = message.indexOf(secret); + start !== -1; + start = message.indexOf(secret, start + 1) + ) { + characters.fill('*', start, start + secret.length); + } + } + return characters.join(''); +} + function createDiscardStream(): Writable { return new Writable({ objectMode: true, @@ -105,13 +112,26 @@ function createDiscardStream(): Writable { export async function createBuildLoggerWithSecretsFilter(secrets: Job['secrets']): Promise<{ logger: bunyan; + addBuildLogMask: (secret: string) => void; cleanUp: () => Promise; logBuffer: WorkerLogBuffer; outputStream: Readable; }> { const buildLogger = defaultLogger.child({ phase: BuildPhase.UNKNOWN }); - const transformStream = createTransformStream(secrets?.environmentSecrets ?? []); + const secretPatterns = new Set(); + const addBuildLogMask = (secret: string): void => { + for (const pattern of [secret, maybeStringBase64Decode(secret)]) { + if (pattern && pattern.length > 1 && !simpleSecretsWhitelist.includes(pattern)) { + secretPatterns.add(pattern); + } + } + }; + for (const { value } of secrets?.environmentSecrets ?? []) { + addBuildLogMask(value); + } + + const transformStream = createTransformStream(secretPatterns); const discardStream = createDiscardStream(); transformStream.pipe(discardStream); @@ -136,7 +156,7 @@ export async function createBuildLoggerWithSecretsFilter(secrets: Job['secrets'] await remoteLoggerStream.init(); } - const logBuffer = new WorkerLogBuffer(MAX_LINES_IN_BUFFER); + const logBuffer = new WorkerLogBuffer(MAX_LINES_IN_BUFFER, secretPatterns); buildLogger.addStream({ type: 'raw', stream: logBuffer, @@ -161,6 +181,7 @@ export async function createBuildLoggerWithSecretsFilter(secrets: Job['secrets'] return { logger: buildLogger, + addBuildLogMask, cleanUp: async () => { await Promise.all([remoteLoggerStream?.cleanUp?.(), httpLogStream?.cleanUp?.()]); }, diff --git a/packages/worker/src/service.ts b/packages/worker/src/service.ts index 7d2b5c80b2..3f62dbeb02 100644 --- a/packages/worker/src/service.ts +++ b/packages/worker/src/service.ts @@ -273,6 +273,7 @@ export default class BuildService { try { const { logger: buildLogger, + addBuildLogMask, cleanUp, logBuffer, } = await createBuildLoggerWithSecretsFilter(job.secrets ?? {}); @@ -300,6 +301,7 @@ export default class BuildService { projectId, buildId: this.buildId, buildLogger, + addBuildLogMask, }); this.buildContext = ctx;