From be4103eebf9f051e479f1c24af4c90021f29a8a1 Mon Sep 17 00:00:00 2001 From: Zbigniew Sobiecki Date: Sun, 22 Feb 2026 16:09:25 +0000 Subject: [PATCH] =?UTF-8?q?refactor(config):=20simplify=20credential=20pas?= =?UTF-8?q?sing=20=E2=80=94=20eliminate=20cache,=20use=20env=20vars=20dire?= =?UTF-8?q?ctly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Credentials are immutable during a worker run. The previous three-mechanism approach (JSON blob env var → parse + spread to process.env → populate cache module → credential functions check env + cache + DB guards) is replaced by a single mechanism: router sets individual env vars, credential functions read from process.env. - Router sets individual env vars + CASCADE_CREDENTIAL_KEYS sentinel - Delete loadRouterCredentials() from worker-entry (env vars set by Docker) - Credential functions use CASCADE_CREDENTIAL_KEYS as worker-context guard - getAllProjectCredentials reconstructs map from env var key list - Remove legacy GITHUB_TOKEN fallback in resolveGitHubToken - Remove dead CASCADE_CREDENTIALS/CASCADE_CREDENTIALS_PROJECT_ID from scrub list Co-Authored-By: Claude Opus 4.6 --- src/backends/secretBuilder.ts | 8 +---- src/config/provider.ts | 33 +++++++++++++++++++ src/router/worker-manager.ts | 11 ++++--- src/utils/envScrub.ts | 3 -- src/worker-entry.ts | 25 +++------------ tests/unit/backends/secretBuilder.test.ts | 16 +++------- tests/unit/config/provider.test.ts | 39 +++++++++++++++++++++++ tests/unit/utils/envScrub.test.ts | 18 ----------- 8 files changed, 87 insertions(+), 66 deletions(-) diff --git a/src/backends/secretBuilder.ts b/src/backends/secretBuilder.ts index 682136ac..1754d549 100644 --- a/src/backends/secretBuilder.ts +++ b/src/backends/secretBuilder.ts @@ -16,13 +16,7 @@ export async function resolveGitHubToken( ): Promise { if (!profile.needsGitHubToken) return undefined; - try { - return await getPersonaToken(projectId, agentType); - } catch { - // Fall back to legacy GITHUB_TOKEN for projects not yet migrated - const secrets = await getAllProjectCredentials(projectId); - return secrets.GITHUB_TOKEN; - } + return getPersonaToken(projectId, agentType); } /** diff --git a/src/config/provider.ts b/src/config/provider.ts index 3c3fa177..175a5a38 100644 --- a/src/config/provider.ts +++ b/src/config/provider.ts @@ -124,6 +124,14 @@ export async function getIntegrationCredential( return process.env[envKey]; } + // Worker context: all credentials set by router, this one doesn't exist + if (process.env.CASCADE_CREDENTIAL_KEYS) { + throw new Error( + `Integration credential '${category}/${role}' not found for project '${projectId}'`, + ); + } + + // Router/dashboard context: resolve from DB const value = await resolveIntegrationCredential(projectId, category, role); if (value) return value; @@ -146,6 +154,12 @@ export async function getIntegrationCredentialOrNull( return process.env[envKey]; } + // Worker context: all credentials set by router, this one doesn't exist + if (process.env.CASCADE_CREDENTIAL_KEYS) { + return null; + } + + // Router/dashboard context: resolve from DB return resolveIntegrationCredential(projectId, category, role); } @@ -166,6 +180,12 @@ export async function getOrgCredential( return process.env[envVarKey]; } + // Worker context: all credentials set by router, this one doesn't exist + if (process.env.CASCADE_CREDENTIAL_KEYS) { + return null; + } + + // Router/dashboard context: resolve from DB const orgId = await getOrgIdForProject(projectId); return resolveOrgCredential(orgId, envVarKey); } @@ -181,6 +201,19 @@ export async function getOrgCredential( * 3. Merges integration credentials over org defaults */ export async function getAllProjectCredentials(projectId: string): Promise> { + // Worker context: reconstruct from individual env vars set by the router + const keyList = process.env.CASCADE_CREDENTIAL_KEYS; + if (keyList) { + const result: Record = {}; + for (const key of keyList.split(',')) { + if (key && process.env[key]) { + result[key] = process.env[key]; + } + } + return result; + } + + // Router/dashboard context: resolve from DB (has CREDENTIAL_MASTER_KEY) const orgId = await getOrgIdForProject(projectId); const [integrationCreds, orgCreds] = await Promise.all([ diff --git a/src/router/worker-manager.ts b/src/router/worker-manager.ts index 4a4f0b22..4db95646 100644 --- a/src/router/worker-manager.ts +++ b/src/router/worker-manager.ts @@ -63,15 +63,16 @@ async function buildWorkerEnv(job: Job): Promise { `LOG_LEVEL=${process.env.LOG_LEVEL || 'info'}`, ]; - // Resolve project credentials in the router and pass as JSON. - // Workers cache these on startup, then scrub from env. + // Resolve project credentials in the router and set as individual env vars. // NOTE: CREDENTIAL_MASTER_KEY is intentionally NOT passed to workers. const projectId = await extractProjectIdFromJob(job.data); if (projectId) { try { const secrets = await getAllProjectCredentials(projectId); - env.push(`CASCADE_CREDENTIALS=${JSON.stringify(secrets)}`); - env.push(`CASCADE_CREDENTIALS_PROJECT_ID=${projectId}`); + for (const [key, value] of Object.entries(secrets)) { + env.push(`${key}=${value}`); + } + env.push(`CASCADE_CREDENTIAL_KEYS=${Object.keys(secrets).join(',')}`); } catch (err) { console.warn('[WorkerManager] Failed to resolve credentials for project:', { projectId, @@ -93,7 +94,7 @@ async function spawnWorker(job: Job): Promise { const containerName = `cascade-worker-${jobId}`; const workerEnv = await buildWorkerEnv(job); - const hasCredentials = workerEnv.some((e) => e.startsWith('CASCADE_CREDENTIALS=')); + const hasCredentials = workerEnv.some((e) => e.startsWith('CASCADE_CREDENTIAL_KEYS=')); console.log('[WorkerManager] Spawning worker:', { jobId, diff --git a/src/utils/envScrub.ts b/src/utils/envScrub.ts index db7d795c..17d78155 100644 --- a/src/utils/envScrub.ts +++ b/src/utils/envScrub.ts @@ -15,8 +15,6 @@ const SENSITIVE_ENV_KEYS = [ 'DATABASE_URL', 'DATABASE_SSL', 'REDIS_URL', - 'CASCADE_CREDENTIALS', - 'CASCADE_CREDENTIALS_PROJECT_ID', ] as const; /** @@ -24,7 +22,6 @@ const SENSITIVE_ENV_KEYS = [ * * Call this AFTER: * - Database connection pool is initialized (getDb()) - * - Project credentials are set as individual env vars (loadRouterCredentials()) * * After scrubbing: * - Database pool continues to work (uses cached connection string) diff --git a/src/worker-entry.ts b/src/worker-entry.ts index 9eb4785a..b28efd85 100644 --- a/src/worker-entry.ts +++ b/src/worker-entry.ts @@ -88,20 +88,6 @@ type DashboardJobData = ManualRunJobData | RetryRunJobData | DebugAnalysisJobDat type JobData = TrelloJobData | GitHubJobData | JiraJobData | DashboardJobData; -function loadRouterCredentials(): void { - const credentialsJson = process.env.CASCADE_CREDENTIALS; - if (!credentialsJson) return; - try { - const secrets = JSON.parse(credentialsJson) as Record; - for (const [key, value] of Object.entries(secrets)) { - process.env[key] = value; - } - logger.info('[Worker] Set credentials as env vars from router'); - } catch (err) { - logger.warn('[Worker] Failed to parse CASCADE_CREDENTIALS', { error: String(err) }); - } -} - async function processDashboardJob(jobId: string, jobData: DashboardJobData): Promise { const { loadProjectConfigById } = await import('./config/provider.js'); @@ -177,18 +163,15 @@ async function main(): Promise { const config = await loadConfig(); logger.info('[Worker] Loaded projects config', { projects: config.projects.map((p) => p.id) }); - // Set credentials as individual env vars (passed as JSON in CASCADE_CREDENTIALS). - // Router resolves and decrypts credentials before spawning workers, so workers - // never need the CREDENTIAL_MASTER_KEY. - if (process.env.CASCADE_CREDENTIALS) { - loadRouterCredentials(); - } else { + // Credentials are set as individual env vars by the router (Docker env). + // CASCADE_CREDENTIAL_KEYS lists the key names for reconstruction. + if (!process.env.CASCADE_CREDENTIAL_KEYS) { logger.error('[Worker] No credentials passed from router - job will likely fail', { jobType: jobData.type, }); } - // SECURITY: Scrub sensitive env vars (DATABASE_URL, CASCADE_CREDENTIALS, etc.) + // SECURITY: Scrub sensitive env vars (DATABASE_URL, etc.) // before agent execution. Subprocesses (Tmux, etc.) will not inherit these secrets. scrubSensitiveEnv(); logger.info('[Worker] Scrubbed sensitive env vars'); diff --git a/tests/unit/backends/secretBuilder.test.ts b/tests/unit/backends/secretBuilder.test.ts index 967e1437..33edb1d6 100644 --- a/tests/unit/backends/secretBuilder.test.ts +++ b/tests/unit/backends/secretBuilder.test.ts @@ -157,19 +157,11 @@ describe('resolveGitHubToken', () => { expect(mockGetPersonaToken).toHaveBeenCalledWith('project-id', 'implementation'); }); - it('falls back to GITHUB_TOKEN from credentials when getPersonaToken throws', async () => { + it('propagates error when getPersonaToken throws', async () => { mockGetPersonaToken.mockRejectedValue(new Error('persona not found')); - mockGetAllProjectCredentials.mockResolvedValue({ GITHUB_TOKEN: 'fallback-token' }); const profile = makeProfile({ needsGitHubToken: true }); - const token = await resolveGitHubToken(profile, 'project-id', 'implementation'); - expect(token).toBe('fallback-token'); - }); - - it('returns undefined from fallback when GITHUB_TOKEN credential is missing', async () => { - mockGetPersonaToken.mockRejectedValue(new Error('persona not found')); - mockGetAllProjectCredentials.mockResolvedValue({}); - const profile = makeProfile({ needsGitHubToken: true }); - const token = await resolveGitHubToken(profile, 'project-id', 'implementation'); - expect(token).toBeUndefined(); + await expect(resolveGitHubToken(profile, 'project-id', 'implementation')).rejects.toThrow( + 'persona not found', + ); }); }); diff --git a/tests/unit/config/provider.test.ts b/tests/unit/config/provider.test.ts index 537af387..efd4986c 100644 --- a/tests/unit/config/provider.test.ts +++ b/tests/unit/config/provider.test.ts @@ -311,6 +311,15 @@ describe('config/provider', () => { "Integration credential 'pm/api_key' not found for project 'proj1'", ); }); + + it('throws without DB fallback when CASCADE_CREDENTIAL_KEYS is set (worker context)', async () => { + setEnvCredential('CASCADE_CREDENTIAL_KEYS', 'OTHER_KEY'); + + await expect(getIntegrationCredential('proj1', 'pm', 'api_key')).rejects.toThrow( + "Integration credential 'pm/api_key' not found for project 'proj1'", + ); + expect(resolveIntegrationCredential).not.toHaveBeenCalled(); + }); }); describe('getIntegrationCredentialOrNull', () => { @@ -337,6 +346,15 @@ describe('config/provider', () => { expect(result).toBe('db-token'); }); + + it('returns null without DB fallback when CASCADE_CREDENTIAL_KEYS is set (worker context)', async () => { + setEnvCredential('CASCADE_CREDENTIAL_KEYS', 'OTHER_KEY'); + + const result = await getIntegrationCredentialOrNull('proj1', 'scm', 'implementer_token'); + + expect(result).toBeNull(); + expect(resolveIntegrationCredential).not.toHaveBeenCalled(); + }); }); describe('getOrgCredential', () => { @@ -376,6 +394,15 @@ describe('config/provider', () => { await expect(getOrgCredential('proj1', 'KEY')).rejects.toThrow('Project not found: proj1'); }); + + it('returns null without DB fallback when CASCADE_CREDENTIAL_KEYS is set (worker context)', async () => { + setEnvCredential('CASCADE_CREDENTIAL_KEYS', 'OTHER_KEY'); + + const result = await getOrgCredential('proj1', 'OPENROUTER_API_KEY'); + + expect(result).toBeNull(); + expect(resolveOrgCredential).not.toHaveBeenCalled(); + }); }); describe('getAllProjectCredentials', () => { @@ -411,6 +438,18 @@ describe('config/provider', () => { const result = await getAllProjectCredentials('proj1'); expect(result).toEqual({}); }); + + it('reconstructs credentials from env vars when CASCADE_CREDENTIAL_KEYS is set (worker context)', async () => { + setEnvCredential('CASCADE_CREDENTIAL_KEYS', 'TRELLO_API_KEY,OPENROUTER_API_KEY'); + setEnvCredential('TRELLO_API_KEY', 'env-key'); + setEnvCredential('OPENROUTER_API_KEY', 'env-or'); + + const result = await getAllProjectCredentials('proj1'); + + expect(result).toEqual({ TRELLO_API_KEY: 'env-key', OPENROUTER_API_KEY: 'env-or' }); + expect(resolveAllIntegrationCredentials).not.toHaveBeenCalled(); + expect(resolveAllOrgCredentials).not.toHaveBeenCalled(); + }); }); describe('invalidateConfigCache', () => { diff --git a/tests/unit/utils/envScrub.test.ts b/tests/unit/utils/envScrub.test.ts index be1c1480..22553102 100644 --- a/tests/unit/utils/envScrub.test.ts +++ b/tests/unit/utils/envScrub.test.ts @@ -12,8 +12,6 @@ describe('scrubSensitiveEnv', () => { DATABASE_URL: process.env.DATABASE_URL, DATABASE_SSL: process.env.DATABASE_SSL, REDIS_URL: process.env.REDIS_URL, - CASCADE_CREDENTIALS: process.env.CASCADE_CREDENTIALS, - CASCADE_CREDENTIALS_PROJECT_ID: process.env.CASCADE_CREDENTIALS_PROJECT_ID, }; }); @@ -52,25 +50,11 @@ describe('scrubSensitiveEnv', () => { expect(process.env.REDIS_URL).toBeUndefined(); }); - it('removes CASCADE_CREDENTIALS from process.env', () => { - process.env.CASCADE_CREDENTIALS = 'eyJzb21lIjoianNvbiJ9'; - scrubSensitiveEnv(); - expect(process.env.CASCADE_CREDENTIALS).toBeUndefined(); - }); - - it('removes CASCADE_CREDENTIALS_PROJECT_ID from process.env', () => { - process.env.CASCADE_CREDENTIALS_PROJECT_ID = 'my-project-id'; - scrubSensitiveEnv(); - expect(process.env.CASCADE_CREDENTIALS_PROJECT_ID).toBeUndefined(); - }); - it('removes all sensitive keys in a single call', () => { process.env.CREDENTIAL_MASTER_KEY = 'key1'; process.env.DATABASE_URL = 'postgres://...'; process.env.DATABASE_SSL = 'true'; process.env.REDIS_URL = 'redis://...'; - process.env.CASCADE_CREDENTIALS = 'creds'; - process.env.CASCADE_CREDENTIALS_PROJECT_ID = 'proj-id'; scrubSensitiveEnv(); @@ -78,8 +62,6 @@ describe('scrubSensitiveEnv', () => { expect(process.env.DATABASE_URL).toBeUndefined(); expect(process.env.DATABASE_SSL).toBeUndefined(); expect(process.env.REDIS_URL).toBeUndefined(); - expect(process.env.CASCADE_CREDENTIALS).toBeUndefined(); - expect(process.env.CASCADE_CREDENTIALS_PROJECT_ID).toBeUndefined(); }); it('does not remove non-sensitive environment variables', () => {