Skip to content

Commit 27cce76

Browse files
sweetmantechclaude
andcommitted
fix(sandbox): SRP split + shellEscape cloneUrl + drop URL from logs
Three review fixes bundled: 1) SRP per sweetmantech: split app/workflows/buildOrgSnapshot.ts into - app/workflows/buildSnapshotStep.ts (the "use step" function) - app/workflows/buildOrgSnapshotWorkflow.ts (the "use workflow" function — filename now matches the export, addressing the cubic P2 about filename mismatch) 2) P0 command injection (cubic): wrap cloneUrl with shellEscape() in the `git clone --depth=1 ...` command. The Zod + parseGitHubRepoUrl validators upstream of this workflow already reject anything that doesn't match `^https:\/\/github\.com\/recoupable\/...`, so this isn't exploitable today — but shell-escaping is defense-in-depth that survives validator changes. Zero behavior change for valid inputs (extra single quotes around an already-clean URL). 3) P1 credential leak (cubic): dropped `cloneUrl` from every log line in the workflow + kick. The `https://user:token@github.com/...` shape is also blocked by the regex validator, but log scrubbing is the cheaper of the two layers if the validator ever loosens. `sandboxName` (the regex-extracted repo name only) remains in logs — uniquely identifies the org for observability without exposing tokens. Out of scope: cubic's P2 about duplicate workflow runs (same fire-and-forget pattern as open-agents — duplicates are wasteful but not incorrect; can add a Vercel Workflow idempotency key in a follow-up if observed in practice). Tests: existing handler tests still pass unchanged (the kick is mocked at the lib path, which didn't change). Suite remains 2577 / 2577. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 60265ee commit 27cce76

4 files changed

Lines changed: 86 additions & 62 deletions

File tree

app/workflows/buildOrgSnapshot.ts

Lines changed: 0 additions & 59 deletions
This file was deleted.
Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import { buildSnapshotStep, type BuildOrgSnapshotInput } from "@/app/workflows/buildSnapshotStep";
2+
3+
/**
4+
* Vercel Workflow that provisions a per-org base snapshot for warm-boot
5+
* of future session sandboxes. Kicked fire-and-forget from
6+
* `createSandboxHandler` when a recoupable org URL is requested but
7+
* no `created` snapshot exists yet.
8+
*
9+
* Single step today (provision + clone + snapshot via `refreshBaseSnapshot`),
10+
* wrapped here for the durable execution semantics — failures retry up
11+
* to 3× automatically, the run is observable in the Vercel dashboard,
12+
* and the request that kicked the workflow is fully decoupled from
13+
* its lifetime.
14+
*/
15+
export async function buildOrgSnapshotWorkflow(input: BuildOrgSnapshotInput) {
16+
"use workflow";
17+
18+
console.log(`[build-org-snapshot] workflow:start name='${input.sandboxName}'`);
19+
20+
try {
21+
const snapshotId = await buildSnapshotStep(input);
22+
console.log(`[build-org-snapshot] Built ${snapshotId} for '${input.sandboxName}'`);
23+
return { success: true as const, snapshotId };
24+
} catch (error) {
25+
const message = error instanceof Error ? error.message : String(error);
26+
console.error(`[build-org-snapshot] Failed for '${input.sandboxName}':`, message);
27+
return { success: false as const, error: message };
28+
}
29+
}

app/workflows/buildSnapshotStep.ts

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { refreshBaseSnapshot } from "@/lib/sandbox/abstraction";
2+
import { getServiceGithubToken } from "@/lib/github/getServiceGithubToken";
3+
import { DEFAULT_SANDBOX_BASE_SNAPSHOT_ID } from "@/lib/sandbox/defaultBaseSnapshotId";
4+
import { shellEscape } from "@/lib/sandbox/shellEscape";
5+
6+
const BUILD_SANDBOX_TIMEOUT_MS = 10 * 60 * 1000; // 10 minutes
7+
const BUILD_COMMAND_TIMEOUT_MS = 8 * 60 * 1000; // 8 minutes — leaves buffer under sandbox timeout
8+
9+
export interface BuildOrgSnapshotInput {
10+
cloneUrl: string;
11+
sandboxName: string;
12+
}
13+
14+
/**
15+
* Single step of `buildOrgSnapshotWorkflow`. Provisions a sandbox from
16+
* the recoup base snapshot, runs `git clone --depth=1 <cloneUrl> .`
17+
* inside it, and snapshots the result. Returns the new snapshot id.
18+
*
19+
* The cloneUrl is shell-escaped before interpolation: the validator
20+
* upstream of this workflow already rejects anything that doesn't
21+
* match `^https:\/\/github\.com\/recoupable\/...`, but defense-in-depth
22+
* — never trust the validator to also be a shell-quoter.
23+
*
24+
* Logging deliberately omits `cloneUrl` to avoid surfacing any token
25+
* embedded as `https://user:token@github.com/...`. The `sandboxName`
26+
* is the regex-extracted repo name only, so it's safe to log.
27+
*/
28+
export async function buildSnapshotStep(input: BuildOrgSnapshotInput): Promise<string> {
29+
"use step";
30+
31+
console.log(`[build-org-snapshot] step:start name='${input.sandboxName}'`);
32+
33+
const githubToken = getServiceGithubToken() ?? undefined;
34+
if (!githubToken) {
35+
throw new Error("[build-org-snapshot] GITHUB_TOKEN is not set; cannot clone org repo");
36+
}
37+
38+
const result = await refreshBaseSnapshot({
39+
baseSnapshotId: DEFAULT_SANDBOX_BASE_SNAPSHOT_ID,
40+
sandboxName: input.sandboxName,
41+
sandboxTimeoutMs: BUILD_SANDBOX_TIMEOUT_MS,
42+
commandTimeoutMs: BUILD_COMMAND_TIMEOUT_MS,
43+
githubToken,
44+
commands: [`git clone --depth=1 ${shellEscape(input.cloneUrl)} .`],
45+
log: message => console.log(`[build-org-snapshot] ${message}`),
46+
});
47+
48+
return result.snapshotId;
49+
}

lib/sandbox/kickBuildOrgSnapshotWorkflow.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { start } from "workflow/api";
2-
import { buildOrgSnapshotWorkflow } from "@/app/workflows/buildOrgSnapshot";
2+
import { buildOrgSnapshotWorkflow } from "@/app/workflows/buildOrgSnapshotWorkflow";
33

44
interface KickBuildOrgSnapshotInput {
55
cloneUrl: string;
@@ -16,6 +16,11 @@ interface KickBuildOrgSnapshotInput {
1616
* falls back to the slow full-clone path; what we're protecting is
1717
* that *future* requests don't have to.
1818
*
19+
* Logging omits `cloneUrl` to avoid surfacing any embedded credential
20+
* (e.g. `https://user:token@github.com/...`) — the `sandboxName` is
21+
* already the regex-extracted repo name only, which uniquely
22+
* identifies the org for observability without exposing tokens.
23+
*
1924
* @param input - The repo URL to clone and the sandbox name to use
2025
* (which becomes the snapshot's name and the lookup key for
2126
* `findOrgSnapshot`).
@@ -24,11 +29,11 @@ export function kickBuildOrgSnapshotWorkflow(input: KickBuildOrgSnapshotInput) {
2429
void start(buildOrgSnapshotWorkflow, [input]).then(
2530
run =>
2631
console.log(
27-
`[build-org-snapshot] Started workflow run ${run.runId} for '${input.sandboxName}' (${input.cloneUrl})`,
32+
`[build-org-snapshot] Started workflow run ${run.runId} for '${input.sandboxName}'`,
2833
),
2934
error =>
3035
console.error(
31-
`[build-org-snapshot] Failed to start workflow for '${input.sandboxName}' (${input.cloneUrl}):`,
36+
`[build-org-snapshot] Failed to start workflow for '${input.sandboxName}':`,
3237
error,
3338
),
3439
);

0 commit comments

Comments
 (0)