fix: address P0-P2 review feedback from Cubic review

P0: Strip OAuth token from git origin after clone so LLM agent
    never sees credentials (repo-agent.tsx)

P1: Replace raw error.message with safe hardcoded text in API
    response and dashboard UI (apply/route.tsx, config-update.tsx)
P1: E2E spike script now requires explicit env vars instead of
    falling back to pushing to main (spike-orchestrator-e2e.mts)

P2: Use urlSchema for commit_url (schema-fields.ts)
P2: Return commitSha directly instead of parsing from URL
    (repo-agent.tsx, apply/route.tsx)
P2: Support LINK_BRANCH_ID env var (link-project-to-github.ts)
P2: Widen structural fallback regex (config-updater.ts)
P2: Log warning when cancel has no sandboxId (cancel/route.tsx)
P2: Reject arbitrary string config values (config-eval.ts)
Co-Authored-By: mantra <mantra@stack-auth.com>
This commit is contained in:
Devin AI 2026-06-25 21:54:46 +00:00
parent 1b6a98ebdd
commit 0f743f93ff
9 changed files with 48 additions and 23 deletions

View File

@ -14,6 +14,7 @@
* LINK_BRANCH=main
* LINK_CONFIG_PATH=hexclave.config.ts
* LINK_WORKFLOW_PATH=.github/workflows/hexclave-config-sync.yml
* LINK_BRANCH_ID=<branch-id> (defaults to DEFAULT_BRANCH_ID)
* LINK_COMMIT=<head sha> (informational; the agent pulls latest)
*/
import { setBranchConfigOverrideSource } from "@/lib/config";
@ -30,9 +31,10 @@ async function main() {
const workflowPath = getEnvVariable("LINK_WORKFLOW_PATH", ".github/workflows/hexclave-config-sync.yml");
const commitHash = getEnvVariable("LINK_COMMIT", "") || "0".repeat(40);
const branchId = getEnvVariable("LINK_BRANCH_ID", "") || DEFAULT_BRANCH_ID;
await setBranchConfigOverrideSource({
projectId,
branchId: DEFAULT_BRANCH_ID,
branchId,
source: {
type: "pushed-from-github",
owner,

View File

@ -21,10 +21,14 @@ import {
type GithubRepoRef,
} from "../src/lib/config/repo-agent";
if (!process.env.SPIKE_OWNER || !process.env.SPIKE_REPO || !process.env.SPIKE_BRANCH) {
console.error("SPIKE_OWNER, SPIKE_REPO, and SPIKE_BRANCH must all be set explicitly.\nThis script pushes commits to a real repo — refusing to fall back to defaults.");
process.exit(1);
}
const REF: GithubRepoRef = {
owner: process.env.SPIKE_OWNER ?? "hexclave",
repo: process.env.SPIKE_REPO ?? "stackframe-website-2026",
branch: process.env.SPIKE_BRANCH ?? "main",
owner: process.env.SPIKE_OWNER,
repo: process.env.SPIKE_REPO,
branch: process.env.SPIKE_BRANCH,
};
// The COMPLETE config we want the repo's config file to reflect (the real flow

View File

@ -123,8 +123,7 @@ export const POST = createSmartRouteHandler({
branchId,
nowMs: Date.now(),
outcome: result.mode === "commit-to-branch"
// The pushed commit sha is the last path segment of the commit URL.
? { status: "success", commitUrl: result.commitUrl, newCommitHash: result.commitUrl.split("/").pop() }
? { status: "success", commitUrl: result.commitUrl, newCommitHash: result.commitSha }
: { status: "no-change" },
});
} catch (error) {
@ -133,7 +132,7 @@ export const POST = createSmartRouteHandler({
projectId,
branchId,
nowMs: Date.now(),
outcome: { status: "error", error: error instanceof Error ? error.message : "The config agent failed to apply the change." },
outcome: { status: "error", error: "The config agent failed to apply the change." },
}).catch((e) => captureError("config-github-apply-record-error", e));
}
});

View File

@ -6,15 +6,17 @@ import { stopConfigAgentSandbox } from "@/lib/config/repo-agent";
import { createSmartRouteHandler } from "@/route-handlers/smart-route-handler";
import { runAsynchronouslyAndWaitUntil } from "@/utils/background-tasks";
import { adaptSchema, adminAuthTypeSchema, yupNumber, yupObject, yupString } from "@hexclave/shared/dist/schema-fields";
import { StatusError } from "@hexclave/shared/dist/utils/errors";
import { StatusError, captureError } from "@hexclave/shared/dist/utils/errors";
export const maxDuration = 60;
/**
* Cancels the in-flight dashboardGitHub config run. Atomically flips the run to
* the terminal `cancelled` status (so the original run's late result is ignored)
* and hard-stops its sandbox. There is no revert: if the agent hadn't pushed yet
* the change is undone; if a commit already landed it stays (and the repo's
* and hard-stops its sandbox when available. If the sandbox id was never recorded
* (e.g. the run failed before booting a sandbox), the status is still flipped but
* no sandbox stop is attempted. There is no revert: if the agent hadn't pushed
* yet the change is undone; if a commit already landed it stays (and the repo's
* config-sync workflow will eventually reconcile it).
*/
export const POST = createSmartRouteHandler({
@ -54,6 +56,8 @@ export const POST = createSmartRouteHandler({
if (sandboxId) {
runAsynchronouslyAndWaitUntil(stopConfigAgentSandbox(sandboxId));
} else {
captureError("config-github-cancel", new Error("Cancelled a config agent run but no sandboxId was recorded; the sandbox may still be running."));
}
return { statusCode: 200, bodyType: "json", body: { status: "cancelling" } };

View File

@ -64,7 +64,7 @@ export type GithubRepoRef = { owner: string, repo: string, branch: string };
export type GithubTokenProvider = () => Promise<string>;
export type ConfigUpdatePushResult =
| { mode: "commit-to-branch", branch: string, commitUrl: string }
| { mode: "commit-to-branch", branch: string, commitUrl: string, commitSha: string }
| { mode: "no-change" };
export class ConfigRepoAgentError extends Error {
@ -140,6 +140,10 @@ function tokenUrl(token: string, ref: Pick<GithubRepoRef, "owner" | "repo">): st
return `https://x-access-token:${token}@github.com/${ref.owner}/${ref.repo}.git`;
}
function tokenlessUrl(ref: Pick<GithubRepoRef, "owner" | "repo">): string {
return `https://github.com/${ref.owner}/${ref.repo}.git`;
}
// ---------------------------------------------------------------------------
// Agent
// ---------------------------------------------------------------------------
@ -413,11 +417,13 @@ export async function applyConfigUpdate(options: {
await run(sandbox, "git", ["config", "--global", "user.email", GIT_BOT_EMAIL]);
await run(sandbox, "git", ["config", "--global", "user.name", GIT_BOT_NAME]);
// Fresh shallow clone of just the target branch. `origin` keeps the tokenized
// URL so the agent's git (and our push) are authenticated; it dies with this
// sandbox, which is never snapshotted, so the token is never persisted.
// Fresh shallow clone of just the target branch. The tokenized URL is used
// only for the clone; immediately after, we reset `origin` to a tokenless URL
// so the agent (which has Bash access) cannot read the token from `.git/config`
// or `git remote -v`. The token is re-injected only for our own push command.
await step(`Cloning ${ref.owner}/${ref.repo}@${ref.branch}`);
await run(sandbox, "git", ["clone", "--depth", "1", "--single-branch", "--branch", ref.branch, tokenUrl(githubToken, ref), REPO_DIR]);
await run(sandbox, "git", ["-C", REPO_DIR, "remote", "set-url", "origin", tokenlessUrl(ref)]);
// Agent writes the COMPLETE config to the file — no dependency install, no
// typecheck (the linked repo's CI validates the committed change). See buildUpdatePrompt.
@ -431,8 +437,9 @@ export async function applyConfigUpdate(options: {
await run(sandbox, "git", ["-C", REPO_DIR, "commit", "-m", commitMessage]);
const commitSha = await gitHead(sandbox);
await step("Pushing the commit…");
await run(sandbox, "git", ["-C", REPO_DIR, "push", "origin", `HEAD:refs/heads/${ref.branch}`]);
return { mode: "commit-to-branch", branch: ref.branch, commitUrl: `https://github.com/${ref.owner}/${ref.repo}/commit/${commitSha}` };
// Re-inject the token for the push only (origin was reset to tokenless after clone).
await run(sandbox, "git", ["-C", REPO_DIR, "push", tokenUrl(githubToken, ref), `HEAD:refs/heads/${ref.branch}`]);
return { mode: "commit-to-branch", branch: ref.branch, commitUrl: `https://github.com/${ref.owner}/${ref.repo}/commit/${commitSha}`, commitSha };
} finally {
await sandbox.stop().catch(() => {});
}

View File

@ -550,7 +550,7 @@ function GithubPushBody({
onRunPhaseChange("idle");
dialogContext?.setGithubRunActive(false);
if (run.status === "error") {
setErrorMessage(run.error ?? "The config agent failed to apply your change.");
setErrorMessage("The config agent failed to apply your change.");
return "prevent-close";
}
if (run.status === "cancelled") {
@ -569,7 +569,6 @@ function GithubPushBody({
setErrorMessage("Timed out waiting for the config agent. Your change may still be in progress — check the linked repository.");
return "prevent-close";
} catch (error) {
const message = error instanceof Error ? error.message : "Unknown error pushing to GitHub.";
captureError("config-update-github-agent", {
projectId,
owner: source.owner,
@ -581,7 +580,7 @@ function GithubPushBody({
setProgressMessage(null);
onRunPhaseChange("idle");
dialogContext?.setGithubRunActive(false);
setErrorMessage(message);
setErrorMessage("Unknown error pushing to GitHub.");
return "prevent-close";
}
}, [adminApp, commitMessage, configUpdate, dialogContext, onRunPhaseChange, onSettle, projectId, scopeCheck, source]);

View File

@ -188,8 +188,9 @@ function configFileExportsConfig(content: string, configFilePath: string): boole
// jiti may fail to resolve imports that are valid in the user's project but
// absent from the current process (e.g. relative asset imports, workspace
// packages). For the structural sanity check we only need to know a runtime
// `config` binding still exists after the agent edited the file.
return /\bexport\s+const\s+config\b/.test(content);
// `config` binding still exists after the agent edited the file. Covers
// `export const config`, `export let config`, `export { config }`, etc.
return /\bexport\s+(?:const|let|var)\s+config\b/.test(content) || /\bexport\s*\{[^}]*\bconfig\b/.test(content);
}
}

View File

@ -70,7 +70,12 @@ export function evalConfigFileContent(content: string, filePath: string): Parsed
if (config === undefined) {
throw new ConfigFileEvalError(`Invalid config in ${filePath}. The file must export a plain \`config\` object or "show-onboarding".`);
}
if (typeof config === "string") return config;
if (typeof config === "string") {
if (config !== "show-onboarding") {
throw new ConfigFileEvalError(`Invalid config in ${filePath}. String config values must be "show-onboarding", got "${config}".`);
}
return config;
}
if (isRecord(config)) return config;
throw new ConfigFileEvalError(`Invalid config in ${filePath}. The file must export a plain \`config\` object or "show-onboarding".`);
}
@ -116,6 +121,10 @@ import.meta.vitest?.test("evalConfigFileContent rejects content without config e
expect(() => evalConfigFileContent("export const other = {};", "stack.config.ts")).toThrow(/must export/);
});
import.meta.vitest?.test("evalConfigFileContent rejects arbitrary string config values", ({ expect }) => {
expect(() => evalConfigFileContent('export const config = "arbitrary-string";', "stack.config.ts")).toThrow(/must be "show-onboarding"/);
});
import.meta.vitest?.test("tryEvalConfigFileContent returns the config for valid exports", ({ expect }) => {
expect(tryEvalConfigFileContent("export const config = { auth: { allowSignUp: true } };", "stack.config.ts")).toEqual({
auth: { allowSignUp: true },

View File

@ -945,7 +945,7 @@ export const branchConfigSourceSchema = yupUnion(
status: yupString().oneOf(["running", "success", "no-change", "error", "cancelled"]).defined(),
started_at: yupNumber().defined(),
finished_at: yupNumber().optional(),
commit_url: yupString().optional(),
commit_url: urlSchema.optional(),
error: yupString().optional(),
// Vercel Sandbox id of the in-flight run, recorded while `status === "running"`
// so a cancel request (a different invocation) can hard-stop the sandbox.