From fea09837002ebb39820847a5d3758e14c5b015b8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 23:34:04 +0000 Subject: [PATCH 1/6] Initial plan From 0af35cd1880a08495f01c8f1b222410bc47d0d31 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 2 Jul 2026 23:39:18 +0000 Subject: [PATCH 2/6] refactor: extract main-action cleanup helpers --- src/commands/main-action.test.ts | 55 ++++++++++- src/commands/main-action.ts | 158 +++++++++++++++++++------------ 2 files changed, 149 insertions(+), 64 deletions(-) diff --git a/src/commands/main-action.test.ts b/src/commands/main-action.test.ts index 1e27e8a73..d3f2599cf 100644 --- a/src/commands/main-action.test.ts +++ b/src/commands/main-action.test.ts @@ -15,7 +15,7 @@ jest.mock('fs', () => { }; }); -import { createMainAction } from './main-action'; +import { createMainAction, testHelpers } from './main-action'; // eslint-disable-next-line @typescript-eslint/no-require-imports jest.mock('../logger', () => require('../test-helpers/mock-logger.test-utils').loggerMockFactory()); @@ -471,4 +471,57 @@ describe('createMainAction', () => { ); }); }); + + describe('extracted helper functions', () => { + it('redactConfigForLogging removes sensitive keys and redacts agentCommand', () => { + const secretValue = 'secret-123'; + const configWithSecrets = { + ...STUB_CONFIG, + agentCommand: `agent --token ${secretValue}`, + openaiApiKey: 'sk-secret', + } as unknown as import('../types').WrapperConfig; + mockedRedactSecrets.redactSecrets.mockImplementation((s: string) => + s.replace(secretValue, '[REDACTED]') + ); + + const redacted = testHelpers.redactConfigForLogging(configWithSecrets); + + expect(redacted).not.toHaveProperty('openaiApiKey'); + expect(redacted.agentCommand).toContain('[REDACTED]'); + expect(redacted.agentCommand).not.toContain(secretValue); + }); + + it('persistConfigAuditArtifact logs debug when writing the artifact fails', () => { + mockMkdirSync.mockImplementationOnce(() => { + throw new Error('write failed'); + }); + + testHelpers.persistConfigAuditArtifact(STUB_CONFIG, { foo: 'bar' }); + + expect(mockedLogger.debug).toHaveBeenCalledWith( + expect.stringContaining('Failed to write resolved config artifact:') + ); + }); + + it('buildCleanupFn runs cleanup using provided state getters', async () => { + const performCleanup = testHelpers.buildCleanupFn( + STUB_CONFIG, + () => true, + () => true, + ); + + await performCleanup(); + + expect(mockedDockerManager.preserveIptablesAudit).toHaveBeenCalledWith( + STUB_CONFIG.workDir, + STUB_CONFIG.auditDir + ); + expect(mockedDockerManager.stopContainers).toHaveBeenCalledWith( + STUB_CONFIG.workDir, + STUB_CONFIG.keepContainers + ); + expect(mockedHostIptables.cleanupHostIptables).toHaveBeenCalled(); + expect(mockedDockerManager.cleanup).toHaveBeenCalled(); + }); + }); }); diff --git a/src/commands/main-action.ts b/src/commands/main-action.ts index eb8405054..568accc6a 100644 --- a/src/commands/main-action.ts +++ b/src/commands/main-action.ts @@ -26,6 +26,86 @@ import { validateOptions } from './validate-options'; import { probeSplitFilesystem } from '../dind-probe'; import { assertTopologySupported, connectTopologyContainers } from '../topology'; import { runDindBootstrap } from '../dind-bootstrap'; +import type { WrapperConfig } from '../types'; + +const SENSITIVE_CONFIG_KEYS = new Set([ + 'openaiApiKey', + 'anthropicApiKey', + 'copilotGithubToken', + 'copilotProviderApiKey', + 'geminiApiKey', +]); + +function redactConfigForLogging(config: WrapperConfig): Record { + const redactedConfig: Record = {}; + for (const [key, value] of Object.entries(config)) { + if (SENSITIVE_CONFIG_KEYS.has(key)) continue; + redactedConfig[key] = key === 'agentCommand' ? redactSecrets(value as string) : value; + } + return redactedConfig; +} + +function persistConfigAuditArtifact( + config: WrapperConfig, + redactedConfig: Record, +): void { + try { + const configArtifactDir = config.auditDir || path.join(config.workDir, 'audit'); + fs.mkdirSync(configArtifactDir, { recursive: true, mode: 0o755 }); + const configArtifactPath = path.join(configArtifactDir, 'awf-resolved-config.json'); + fs.writeFileSync( + configArtifactPath, + JSON.stringify(redactedConfig, null, 2) + '\n', + { mode: 0o644 }, + ); + fs.chmodSync(configArtifactPath, 0o644); + } catch (err) { + logger.debug(`Failed to write resolved config artifact: ${err}`); + } +} + +function buildCleanupFn( + config: WrapperConfig, + getContainersStarted: () => boolean, + getHostIptablesSetup: () => boolean, +) { + return async (signal?: string) => { + if (signal) { + logger.info(`Received ${signal}, cleaning up...`); + } + + // Copy iptables audit BEFORE stopping containers (volumes are destroyed by `docker compose down -v`) + if (getContainersStarted()) { + preserveIptablesAudit(config.workDir, config.auditDir); + await stopContainers(config.workDir, config.keepContainers); + } + + if (getHostIptablesSetup() && !config.keepContainers) { + await cleanupHostIptables(); + } + + if (!config.keepContainers) { + await cleanup( + config.workDir, + false, + config.proxyLogsDir, + config.auditDir, + config.sessionStateDir, + config.dockerHostPathPrefix, + config.imageRegistry, + config.imageTag, + config.agentImage, + ); + // Note: We don't remove the firewall network here since it can be reused + // across multiple runs. Cleanup script will handle removal if needed. + } else { + logger.info(`Configuration files preserved at: ${config.workDir}`); + logger.info(`Agent logs available at: ${config.workDir}/agent-logs/`); + logger.info(`Squid logs available at: ${config.workDir}/squid-logs/`); + logger.info(`Host iptables rules preserved (--keep-containers enabled)`); + } + }; +} /** * Resolves the Commander option-value source for a given option name. @@ -106,33 +186,9 @@ export function createMainAction(getOptionValueSource: OptionSourceResolver) { // Log config with redacted secrets - remove API keys entirely // to prevent sensitive data from flowing to logger (CodeQL sensitive data logging) - const redactedConfig: Record = {}; - for (const [key, value] of Object.entries(config)) { - if ( - key === 'openaiApiKey' || - key === 'anthropicApiKey' || - key === 'copilotGithubToken' || - key === 'copilotProviderApiKey' || - key === 'geminiApiKey' - ) continue; - redactedConfig[key] = key === 'agentCommand' ? redactSecrets(value as string) : value; - } + const redactedConfig = redactConfigForLogging(config); logger.debug('Configuration:', JSON.stringify(redactedConfig, null, 2)); - - // Persist redacted config to audit artifact for post-run diagnostics - try { - const configArtifactDir = config.auditDir || path.join(config.workDir, 'audit'); - fs.mkdirSync(configArtifactDir, { recursive: true, mode: 0o755 }); - const configArtifactPath = path.join(configArtifactDir, 'awf-resolved-config.json'); - fs.writeFileSync( - configArtifactPath, - JSON.stringify(redactedConfig, null, 2) + '\n', - { mode: 0o644 }, - ); - fs.chmodSync(configArtifactPath, 0o644); - } catch (err) { - logger.debug(`Failed to write resolved config artifact: ${err}`); - } + persistConfigAuditArtifact(config, redactedConfig); logger.info(`Allowed domains: ${config.allowedDomains.join(', ')}`); if (config.blockedDomains && config.blockedDomains.length > 0) { @@ -145,43 +201,11 @@ export function createMainAction(getOptionValueSource: OptionSourceResolver) { let containersStarted = false; let hostIptablesSetup = false; - // Handle cleanup on process exit - const performCleanup = async (signal?: string) => { - if (signal) { - logger.info(`Received ${signal}, cleaning up...`); - } - - // Copy iptables audit BEFORE stopping containers (volumes are destroyed by `docker compose down -v`) - if (containersStarted) { - preserveIptablesAudit(config.workDir, config.auditDir); - await stopContainers(config.workDir, config.keepContainers); - } - - if (hostIptablesSetup && !config.keepContainers) { - await cleanupHostIptables(); - } - - if (!config.keepContainers) { - await cleanup( - config.workDir, - false, - config.proxyLogsDir, - config.auditDir, - config.sessionStateDir, - config.dockerHostPathPrefix, - config.imageRegistry, - config.imageTag, - config.agentImage, - ); - // Note: We don't remove the firewall network here since it can be reused - // across multiple runs. Cleanup script will handle removal if needed. - } else { - logger.info(`Configuration files preserved at: ${config.workDir}`); - logger.info(`Agent logs available at: ${config.workDir}/agent-logs/`); - logger.info(`Squid logs available at: ${config.workDir}/squid-logs/`); - logger.info(`Host iptables rules preserved (--keep-containers enabled)`); - } - }; + const performCleanup = buildCleanupFn( + config, + () => containersStarted, + () => hostIptablesSetup, + ); // Register signal handlers for graceful shutdown registerSignalHandlers({ @@ -226,3 +250,11 @@ export function createMainAction(getOptionValueSource: OptionSourceResolver) { } }; } + +/** @internal Exposed for unit tests. */ +// ts-prune-ignore-next +export const testHelpers = { + redactConfigForLogging, + persistConfigAuditArtifact, + buildCleanupFn, +}; From 0cc34c5e0e1261e4ad0229f59346d50f250878bd Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 2 Jul 2026 20:33:31 -0700 Subject: [PATCH 3/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/commands/main-action.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/commands/main-action.ts b/src/commands/main-action.ts index 568accc6a..458b846b7 100644 --- a/src/commands/main-action.ts +++ b/src/commands/main-action.ts @@ -34,6 +34,7 @@ const SENSITIVE_CONFIG_KEYS = new Set([ 'copilotGithubToken', 'copilotProviderApiKey', 'geminiApiKey', + 'githubToken', ]); function redactConfigForLogging(config: WrapperConfig): Record { From 0f2f9c2d68cb23de1ab69d4075a93eaec6eaba9a Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 2 Jul 2026 20:33:38 -0700 Subject: [PATCH 4/6] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/commands/main-action.ts | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/commands/main-action.ts b/src/commands/main-action.ts index 458b846b7..8b914a20c 100644 --- a/src/commands/main-action.ts +++ b/src/commands/main-action.ts @@ -41,9 +41,21 @@ function redactConfigForLogging(config: WrapperConfig): Record const redactedConfig: Record = {}; for (const [key, value] of Object.entries(config)) { if (SENSITIVE_CONFIG_KEYS.has(key)) continue; - redactedConfig[key] = key === 'agentCommand' ? redactSecrets(value as string) : value; + + if (key === 'agentCommand') { + redactedConfig[key] = redactSecrets(value as string); + continue; + } + + if (key === 'additionalEnv' && value && typeof value === 'object') { + redactedConfig[key] = Object.fromEntries( + Object.keys(value as Record).map((envKey) => [envKey, '[REDACTED]']), + ); + continue; + } + + redactedConfig[key] = value; } - return redactedConfig; } function persistConfigAuditArtifact( From 8d11a0ef7c9b11c9e92f4b65e41c9cb18f1dba8b Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Thu, 2 Jul 2026 20:40:52 -0700 Subject: [PATCH 5/6] Potential fix for pull request finding 'CodeQL / Insecure temporary file' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- src/commands/main-action.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/commands/main-action.ts b/src/commands/main-action.ts index 8b914a20c..6cd6eb0b9 100644 --- a/src/commands/main-action.ts +++ b/src/commands/main-action.ts @@ -64,14 +64,14 @@ function persistConfigAuditArtifact( ): void { try { const configArtifactDir = config.auditDir || path.join(config.workDir, 'audit'); - fs.mkdirSync(configArtifactDir, { recursive: true, mode: 0o755 }); + fs.mkdirSync(configArtifactDir, { recursive: true, mode: 0o700 }); const configArtifactPath = path.join(configArtifactDir, 'awf-resolved-config.json'); - fs.writeFileSync( - configArtifactPath, - JSON.stringify(redactedConfig, null, 2) + '\n', - { mode: 0o644 }, - ); - fs.chmodSync(configArtifactPath, 0o644); + const fd = fs.openSync(configArtifactPath, 'wx', 0o600); + try { + fs.writeFileSync(fd, JSON.stringify(redactedConfig, null, 2) + '\n'); + } finally { + fs.closeSync(fd); + } } catch (err) { logger.debug(`Failed to write resolved config artifact: ${err}`); } From 034f684653bbf84b1531917c2cb015c53a086978 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 3 Jul 2026 03:53:07 +0000 Subject: [PATCH 6/6] fix: add missing return in redactConfigForLogging and sync tests --- src/commands/main-action.test.ts | 26 ++++++++++++++++++-------- src/commands/main-action.ts | 1 + 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/commands/main-action.test.ts b/src/commands/main-action.test.ts index d3f2599cf..e001d216f 100644 --- a/src/commands/main-action.test.ts +++ b/src/commands/main-action.test.ts @@ -4,6 +4,8 @@ const mockMkdirSync = jest.fn(); const mockWriteFileSync = jest.fn(); const mockChmodSync = jest.fn(); +const mockOpenSync = jest.fn().mockReturnValue(42); +const mockCloseSync = jest.fn(); jest.mock('fs', () => { const actual = jest.requireActual('fs'); @@ -12,6 +14,8 @@ jest.mock('fs', () => { mkdirSync: (...args: unknown[]) => mockMkdirSync(...args), writeFileSync: (...args: unknown[]) => mockWriteFileSync(...args), chmodSync: (...args: unknown[]) => mockChmodSync(...args), + openSync: (...args: unknown[]) => mockOpenSync(...args), + closeSync: (...args: unknown[]) => mockCloseSync(...args), }; }); @@ -400,6 +404,9 @@ describe('createMainAction', () => { mockMkdirSync.mockReset(); mockWriteFileSync.mockReset(); mockChmodSync.mockReset(); + mockOpenSync.mockReset(); + mockOpenSync.mockReturnValue(42); + mockCloseSync.mockReset(); }); afterEach(() => jest.restoreAllMocks()); @@ -415,16 +422,15 @@ describe('createMainAction', () => { const action = createMainAction(getOptionValueSource); await action(['echo hi'], {}); - expect(mockMkdirSync).toHaveBeenCalledWith('/tmp/awf-audit', { recursive: true, mode: 0o755 }); + expect(mockMkdirSync).toHaveBeenCalledWith('/tmp/awf-audit', { recursive: true, mode: 0o700 }); + expect(mockOpenSync).toHaveBeenCalledWith('/tmp/awf-audit/awf-resolved-config.json', 'wx', 0o600); expect(mockWriteFileSync).toHaveBeenCalledWith( - '/tmp/awf-audit/awf-resolved-config.json', + 42, expect.stringContaining('"allowedDomains"'), - { mode: 0o644 }, ); - expect(mockChmodSync).toHaveBeenCalledWith('/tmp/awf-audit/awf-resolved-config.json', 0o644); // Verify secret key names are excluded from the artifact const written = mockWriteFileSync.mock.calls.find( - (c) => String(c[0]).includes('awf-resolved-config.json') + (c) => typeof c[0] === 'number' ); expect(written).toBeDefined(); const writtenJson = String(written![1]); @@ -451,7 +457,7 @@ describe('createMainAction', () => { await action(['echo hi'], {}); const written = mockWriteFileSync.mock.calls.find( - (c) => String(c[0]).includes('awf-resolved-config.json') + (c) => typeof c[0] === 'number' ); expect(written).toBeDefined(); const writtenJson = String(written![1]); @@ -464,10 +470,14 @@ describe('createMainAction', () => { const action = createMainAction(getOptionValueSource); await action(['echo hi'], {}); - expect(mockWriteFileSync).toHaveBeenCalledWith( + expect(mockOpenSync).toHaveBeenCalledWith( '/tmp/awf-test/audit/awf-resolved-config.json', + 'wx', + 0o600, + ); + expect(mockWriteFileSync).toHaveBeenCalledWith( + 42, expect.any(String), - { mode: 0o644 }, ); }); }); diff --git a/src/commands/main-action.ts b/src/commands/main-action.ts index 6cd6eb0b9..9fae0698d 100644 --- a/src/commands/main-action.ts +++ b/src/commands/main-action.ts @@ -56,6 +56,7 @@ function redactConfigForLogging(config: WrapperConfig): Record redactedConfig[key] = value; } + return redactedConfig; } function persistConfigAuditArtifact(