Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/run-agent-secrets-filter.md
Original file line number Diff line number Diff line change
@@ -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.
3 changes: 3 additions & 0 deletions packages/build-tools/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ export interface BuildContextOptions {
metadata?: Metadata;
expoApiV2BaseUrl?: string;
mcpServerUrl?: string;
addBuildLogMask?: (secret: string) => void;
}

export class SkipNativeBuildError extends Error {}
Expand All @@ -86,6 +87,7 @@ export class BuildContext<TJob extends Job = Job> {
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;
Expand Down Expand Up @@ -115,6 +117,7 @@ export class BuildContext<TJob extends Job = Job> {
this.skipNativeBuild = options.skipNativeBuild;
this.expoApiV2BaseUrl = options.expoApiV2BaseUrl;
this.mcpServerUrl = options.mcpServerUrl;
this.addBuildLogMask = options.addBuildLogMask;

const environmentSecrets = this.getEnvironmentSecrets(job);
this._env = {
Expand Down
2 changes: 2 additions & 0 deletions packages/build-tools/src/customBuildContext.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ export interface BuilderRuntimeApi {
uploadArtifact: (spec: { artifact: ArtifactToUpload; logger: bunyan }) => Promise<{
artifactId: string | null;
}>;
addBuildLogMask?: (secret: string) => void;
}

export class CustomBuildContext<TJob extends Job = Job> implements ExternalBuildContextProvider {
Expand Down Expand Up @@ -84,6 +85,7 @@ export class CustomBuildContext<TJob extends Job = Job> 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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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' },
Expand Down Expand Up @@ -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'],
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 4 additions & 0 deletions packages/build-tools/src/steps/functions/runAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand All @@ -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,
Expand Down
74 changes: 55 additions & 19 deletions packages/build-tools/src/steps/utils/__tests__/agentRun.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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);
},
});
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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 }));
Expand All @@ -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 () => {
Expand Down
14 changes: 5 additions & 9 deletions packages/build-tools/src/steps/utils/agentRun.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ export async function runAgentAsync({
pathEnv,
npmCacheUrl,
logger,
addBuildLogMask,
signal,
}: {
agentKind: AgentKind;
Expand All @@ -47,11 +48,11 @@ export async function runAgentAsync({
pathEnv: string;
npmCacheUrl?: string;
logger: bunyan;
addBuildLogMask: (secret: string) => void;
signal: AbortSignal;
}): Promise<void> {
const leaseOptions = { expoApiV2BaseUrl, expoToken, agentRunId, signal };
let credentials = await leaseAgentRunProviderCredentialsAsync(leaseOptions);
const secrets = new Set<string>();

const runDirectory = await fs.mkdtemp(path.join(os.tmpdir(), 'eas-agent-run-'));
try {
Expand Down Expand Up @@ -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,
});
Expand Down Expand Up @@ -170,7 +170,6 @@ async function runAgentProcessAsync({
cwd,
stdin,
formatOutputLine,
secrets,
logger,
signal,
}: {
Expand All @@ -180,16 +179,13 @@ async function runAgentProcessAsync({
cwd: string;
stdin: string;
formatOutputLine: AgentCli['formatOutputLine'];
secrets: string[];
logger: bunyan;
signal: AbortSignal;
}): Promise<{ exitCode: number | null; exitSignal: NodeJS.Signals | null }> {
signal.throwIfAborted();
// 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<void>(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[];
Expand All @@ -201,12 +197,12 @@ async function runAgentProcessAsync({
return;
}
for (const { level, message } of messages) {
stdoutLogger[level](redact(message).trimEnd());
stdoutLogger[level](message.trimEnd());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: while reviewing that also found that the default stdout logger that the worker users supposedly does not mask secrets

}
});
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', () => {});
Expand Down
1 change: 1 addition & 0 deletions packages/worker/src/__unit__/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ describe(createBuildContext.name, () => {
projectId: 'project-id',
buildId: 'build-id',
buildLogger,
addBuildLogMask: jest.fn(),
};

beforeEach(() => {
Expand Down
50 changes: 50 additions & 0 deletions packages/worker/src/__unit__/logger.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({});

Expand Down
1 change: 1 addition & 0 deletions packages/worker/src/__unit__/service.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
3 changes: 3 additions & 0 deletions packages/worker/src/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ export async function createBuildContext<TJob extends Job>({
projectId,
buildId,
buildLogger,
addBuildLogMask,
}: {
job: TJob;
logBuffer: LogBuffer;
Expand All @@ -34,6 +35,7 @@ export async function createBuildContext<TJob extends Job>({
projectId: string;
buildId: string;
buildLogger: bunyan;
addBuildLogMask: (secret: string) => void;
}): Promise<BuildContext<TJob>> {
const childLogger = buildLogger.child({ buildId });
await RuntimeSettings.loadAsync({
Expand Down Expand Up @@ -104,6 +106,7 @@ export async function createBuildContext<TJob extends Job>({
metadata,
expoApiV2BaseUrl: config.wwwApiV2BaseUrl,
mcpServerUrl: config.mcpServerUrl,
addBuildLogMask,
});
return ctx;
}
Loading
Loading