fix(worker): materialize env files 0600, re-materialize per run, delete on stop
This commit is contained in:
@@ -0,0 +1,35 @@
|
|||||||
|
import { mkdir, rm, writeFile } from 'node:fs/promises';
|
||||||
|
import path from 'node:path';
|
||||||
|
|
||||||
|
import { assertContainedRealPath } from './path-containment';
|
||||||
|
|
||||||
|
export type MaterializedSecret = { name: string; value: string };
|
||||||
|
|
||||||
|
// Format matches the historical `NAME="value"` env-file layout the agent CLIs
|
||||||
|
// source; values are JSON-quoted so newlines/quotes survive.
|
||||||
|
export const formatMaterializedEnv = (secrets: MaterializedSecret[]) =>
|
||||||
|
`${secrets
|
||||||
|
.map((secret) => `${secret.name}=${JSON.stringify(secret.value)}`)
|
||||||
|
.join('\n')}\n`;
|
||||||
|
|
||||||
|
// Resolves the contained env-file path inside the checkout and writes it 0600
|
||||||
|
// so plaintext secrets never sit world/group-readable in the persistent home.
|
||||||
|
export const writeMaterializedEnv = async (
|
||||||
|
repoDir: string,
|
||||||
|
envFilePath: string,
|
||||||
|
secrets: MaterializedSecret[],
|
||||||
|
): Promise<string> => {
|
||||||
|
const envPath = await assertContainedRealPath(repoDir, envFilePath, {
|
||||||
|
forWrite: true,
|
||||||
|
});
|
||||||
|
await mkdir(path.dirname(envPath), { recursive: true });
|
||||||
|
await writeFile(envPath, formatMaterializedEnv(secrets), { mode: 0o600 });
|
||||||
|
return envPath;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Removes a previously materialized env file; no-ops when the path is unset or
|
||||||
|
// already gone so it is safe on every teardown path.
|
||||||
|
export const removeMaterializedEnv = async (envPath: string | undefined) => {
|
||||||
|
if (!envPath) return;
|
||||||
|
await rm(envPath, { force: true });
|
||||||
|
};
|
||||||
@@ -30,6 +30,7 @@ import {
|
|||||||
} from './git';
|
} from './git';
|
||||||
import { getInstallationToken, openDraftPullRequest } from './github';
|
import { getInstallationToken, openDraftPullRequest } from './github';
|
||||||
import { createHeartbeatController } from './heartbeat-controller';
|
import { createHeartbeatController } from './heartbeat-controller';
|
||||||
|
import { removeMaterializedEnv, writeMaterializedEnv } from './materialize-env';
|
||||||
import { assertContainedRealPath } from './path-containment';
|
import { assertContainedRealPath } from './path-containment';
|
||||||
import { createRedactor, truncate } from './redact';
|
import { createRedactor, truncate } from './redact';
|
||||||
import {
|
import {
|
||||||
@@ -50,6 +51,8 @@ type ActiveWorkspace = AdapterWorkspace & {
|
|||||||
cancelRequested?: boolean;
|
cancelRequested?: boolean;
|
||||||
resolveTurn?: () => void;
|
resolveTurn?: () => void;
|
||||||
lastRecordedDiffSignature?: string;
|
lastRecordedDiffSignature?: string;
|
||||||
|
// Absolute path of the last materialized env file so teardown can delete it.
|
||||||
|
materializedEnvPath?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
type FileTreeNode = {
|
type FileTreeNode = {
|
||||||
@@ -717,16 +720,11 @@ const recordChangedFiles = async (
|
|||||||
const materializeEnvFile = async (workspace: ActiveWorkspace) => {
|
const materializeEnvFile = async (workspace: ActiveWorkspace) => {
|
||||||
const { claim, repoDir } = workspace;
|
const { claim, repoDir } = workspace;
|
||||||
if (!claim.job.materializeEnvFile || !claim.job.envFilePath) return;
|
if (!claim.job.materializeEnvFile || !claim.job.envFilePath) return;
|
||||||
const envPath = await assertContainedRealPath(
|
workspace.materializedEnvPath = await writeMaterializedEnv(
|
||||||
repoDir,
|
repoDir,
|
||||||
claim.job.envFilePath,
|
claim.job.envFilePath,
|
||||||
{ forWrite: true },
|
claim.secrets,
|
||||||
);
|
);
|
||||||
await mkdir(path.dirname(envPath), { recursive: true });
|
|
||||||
const content = `${claim.secrets
|
|
||||||
.map((secret) => `${secret.name}=${JSON.stringify(secret.value)}`)
|
|
||||||
.join('\n')}\n`;
|
|
||||||
await writeFile(envPath, content);
|
|
||||||
await appendEvent(
|
await appendEvent(
|
||||||
claim.job._id,
|
claim.job._id,
|
||||||
'info',
|
'info',
|
||||||
@@ -1194,6 +1192,9 @@ export const sendWorkspaceMessage = async (
|
|||||||
if (workspace.agentTurnActive) {
|
if (workspace.agentTurnActive) {
|
||||||
throw new Error('Wait for the current agent turn to finish or abort it.');
|
throw new Error('Wait for the current agent turn to finish or abort it.');
|
||||||
}
|
}
|
||||||
|
// Re-materialize the env file fresh each turn so it is present for the
|
||||||
|
// adapter and can be removed between turns / on teardown.
|
||||||
|
await materializeEnvFile(workspace);
|
||||||
if (options.recordUserMessage ?? true) {
|
if (options.recordUserMessage ?? true) {
|
||||||
await appendMessage({
|
await appendMessage({
|
||||||
jobId: claim.job._id,
|
jobId: claim.job._id,
|
||||||
@@ -1376,6 +1377,8 @@ const teardownActiveWorkspace = async (args: {
|
|||||||
abortAgent: args.abortAgent
|
abortAgent: args.abortAgent
|
||||||
? async () => await abortWorkspaceAgentForWorkspace(workspace)
|
? async () => await abortWorkspaceAgentForWorkspace(workspace)
|
||||||
: undefined,
|
: undefined,
|
||||||
|
removeMaterializedEnv: async () =>
|
||||||
|
await removeMaterializedEnv(workspace.materializedEnvPath),
|
||||||
markStopped: async () =>
|
markStopped: async () =>
|
||||||
await markWorkspaceStopped(workspace.claim.job._id, args.workspaceStatus),
|
await markWorkspaceStopped(workspace.claim.job._id, args.workspaceStatus),
|
||||||
release: () => workspace.boxHandle.release(),
|
release: () => workspace.boxHandle.release(),
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ export const teardownWorkspace = async (steps: {
|
|||||||
removeActive: TeardownStep;
|
removeActive: TeardownStep;
|
||||||
appendEvent?: TeardownStep;
|
appendEvent?: TeardownStep;
|
||||||
abortAgent?: TeardownStep;
|
abortAgent?: TeardownStep;
|
||||||
|
removeMaterializedEnv?: TeardownStep;
|
||||||
markStopped?: TeardownStep;
|
markStopped?: TeardownStep;
|
||||||
closeAgent?: TeardownStep;
|
closeAgent?: TeardownStep;
|
||||||
stopContainer?: TeardownStep;
|
stopContainer?: TeardownStep;
|
||||||
@@ -27,6 +28,7 @@ export const teardownWorkspace = async (steps: {
|
|||||||
await Promise.all(immediate);
|
await Promise.all(immediate);
|
||||||
await call(steps.appendEvent);
|
await call(steps.appendEvent);
|
||||||
await call(steps.abortAgent);
|
await call(steps.abortAgent);
|
||||||
|
await call(steps.removeMaterializedEnv);
|
||||||
await call(steps.markStopped);
|
await call(steps.markStopped);
|
||||||
await call(steps.closeAgent);
|
await call(steps.closeAgent);
|
||||||
await call(steps.stopContainer);
|
await call(steps.stopContainer);
|
||||||
|
|||||||
@@ -0,0 +1,58 @@
|
|||||||
|
import { access, mkdtemp, readFile, rm, stat } from 'node:fs/promises';
|
||||||
|
import os from 'node:os';
|
||||||
|
import path from 'node:path';
|
||||||
|
import { afterEach, describe, expect, test } from 'vitest';
|
||||||
|
|
||||||
|
import {
|
||||||
|
removeMaterializedEnv,
|
||||||
|
writeMaterializedEnv,
|
||||||
|
} from '../../src/materialize-env';
|
||||||
|
|
||||||
|
const tempDirs: string[] = [];
|
||||||
|
|
||||||
|
const mode = async (filePath: string) => (await stat(filePath)).mode & 0o777;
|
||||||
|
|
||||||
|
const exists = async (filePath: string) =>
|
||||||
|
access(filePath).then(
|
||||||
|
() => true,
|
||||||
|
() => false,
|
||||||
|
);
|
||||||
|
|
||||||
|
describe('materialize env', () => {
|
||||||
|
afterEach(async () => {
|
||||||
|
await Promise.all(
|
||||||
|
tempDirs.map((dir) => rm(dir, { force: true, recursive: true })),
|
||||||
|
);
|
||||||
|
tempDirs.length = 0;
|
||||||
|
});
|
||||||
|
|
||||||
|
test('writes the env file 0600 at the returned path with quoted values', async () => {
|
||||||
|
const repoDir = await mkdtemp(path.join(os.tmpdir(), 'spoon-env-'));
|
||||||
|
tempDirs.push(repoDir);
|
||||||
|
|
||||||
|
const envPath = await writeMaterializedEnv(repoDir, '.env.local', [
|
||||||
|
{ name: 'A', value: 'x' },
|
||||||
|
]);
|
||||||
|
|
||||||
|
await expect(exists(envPath)).resolves.toBe(true);
|
||||||
|
await expect(mode(envPath)).resolves.toBe(0o600);
|
||||||
|
await expect(readFile(envPath, 'utf8')).resolves.toBe('A="x"\n');
|
||||||
|
});
|
||||||
|
|
||||||
|
test('removeMaterializedEnv deletes an existing file and no-ops when absent', async () => {
|
||||||
|
const repoDir = await mkdtemp(path.join(os.tmpdir(), 'spoon-env-'));
|
||||||
|
tempDirs.push(repoDir);
|
||||||
|
|
||||||
|
const envPath = await writeMaterializedEnv(repoDir, '.env.local', [
|
||||||
|
{ name: 'A', value: 'x' },
|
||||||
|
]);
|
||||||
|
await expect(exists(envPath)).resolves.toBe(true);
|
||||||
|
|
||||||
|
await removeMaterializedEnv(envPath);
|
||||||
|
await expect(exists(envPath)).resolves.toBe(false);
|
||||||
|
|
||||||
|
// No-ops when the file is already gone or the path is undefined.
|
||||||
|
await expect(removeMaterializedEnv(envPath)).resolves.toBeUndefined();
|
||||||
|
await expect(removeMaterializedEnv(undefined)).resolves.toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user