mirror of
https://github.com/OpenRouterTeam/spawn.git
synced 2026-05-22 03:14:57 +00:00
fix: fail fast when GCP delete is missing project metadata (#2925)
When history metadata lacks a project ID, spawn delete silently fell back to the gcloud default project, attempting deletion in the wrong project (404) while the instance kept running and billing. Now fails fast with a clear error and link to GCP Console. Also adds a defensive check in destroyInstance() to reject empty project. Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: L <6723574+louisgv@users.noreply.github.com>
This commit is contained in:
parent
f5f0b9ec64
commit
56f7840f0c
3 changed files with 73 additions and 20 deletions
|
|
@ -380,4 +380,50 @@ describe("confirmAndDelete edge cases", () => {
|
|||
const result = await confirmAndDelete(record, mockManifest, handler);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it("fails fast when GCP record is missing project metadata", async () => {
|
||||
clack.confirm.mockResolvedValue(true);
|
||||
const record = makeRecord({
|
||||
cloud: "gcp",
|
||||
connection: {
|
||||
ip: "10.0.0.1",
|
||||
user: "root",
|
||||
server_name: "spawn-gcp-123",
|
||||
server_id: "gcp-123",
|
||||
cloud: "gcp",
|
||||
metadata: {
|
||||
zone: "us-central1-a",
|
||||
// project intentionally omitted
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// ensureDeleteCredentials throws before the spinner starts,
|
||||
// so the error propagates as a rejection from confirmAndDelete
|
||||
await expect(confirmAndDelete(record, mockManifest)).rejects.toThrow("Cannot determine GCP project");
|
||||
});
|
||||
|
||||
it("succeeds when GCP record has project metadata", async () => {
|
||||
clack.confirm.mockResolvedValue(true);
|
||||
// With a custom handler that simulates successful deletion,
|
||||
// the project metadata path should not throw
|
||||
const handler = mock(async () => true);
|
||||
const record = makeRecord({
|
||||
cloud: "gcp",
|
||||
connection: {
|
||||
ip: "10.0.0.1",
|
||||
user: "root",
|
||||
server_name: "spawn-gcp-456",
|
||||
server_id: "gcp-456",
|
||||
cloud: "gcp",
|
||||
metadata: {
|
||||
zone: "us-central1-a",
|
||||
project: "my-gcp-project",
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
const result = await confirmAndDelete(record, mockManifest, handler);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -43,14 +43,19 @@ async function ensureDeleteCredentials(record: SpawnRecord): Promise<void> {
|
|||
case "gcp": {
|
||||
const zone = conn.metadata?.zone || "us-central1-a";
|
||||
const project = conn.metadata?.project || "";
|
||||
if (!project) {
|
||||
throw new Error(
|
||||
"Cannot determine GCP project for this instance.\n\n" +
|
||||
"The history entry is missing project metadata. Without it, deletion\n" +
|
||||
"could target the wrong project.\n\n" +
|
||||
"To fix: delete the instance manually from the GCP Console:\n" +
|
||||
" https://console.cloud.google.com/compute/instances",
|
||||
);
|
||||
}
|
||||
validateMetadataValue(zone, "GCP zone");
|
||||
if (project) {
|
||||
validateMetadataValue(project, "GCP project");
|
||||
}
|
||||
validateMetadataValue(project, "GCP project");
|
||||
process.env.GCP_ZONE = zone;
|
||||
if (project) {
|
||||
process.env.GCP_PROJECT = project;
|
||||
}
|
||||
process.env.GCP_PROJECT = project;
|
||||
await gcpEnsureGcloudCli();
|
||||
await gcpAuthenticate();
|
||||
break;
|
||||
|
|
@ -125,27 +130,25 @@ async function execDeleteServer(record: SpawnRecord): Promise<boolean> {
|
|||
case "gcp": {
|
||||
const zone = conn.metadata?.zone || "us-central1-a";
|
||||
const project = conn.metadata?.project || "";
|
||||
if (!project) {
|
||||
throw new Error(
|
||||
"Cannot determine GCP project for this instance.\n\n" +
|
||||
"The history entry is missing project metadata. Without it, deletion\n" +
|
||||
"could target the wrong project.\n\n" +
|
||||
"To fix: delete the instance manually from the GCP Console:\n" +
|
||||
" https://console.cloud.google.com/compute/instances",
|
||||
);
|
||||
}
|
||||
// SECURITY: Validate metadata values to prevent injection via tampered history
|
||||
validateMetadataValue(zone, "GCP zone");
|
||||
if (project) {
|
||||
validateMetadataValue(project, "GCP project");
|
||||
}
|
||||
validateMetadataValue(project, "GCP project");
|
||||
return tryDelete(async () => {
|
||||
process.env.GCP_ZONE = zone;
|
||||
if (project) {
|
||||
process.env.GCP_PROJECT = project;
|
||||
}
|
||||
process.env.GCP_PROJECT = project;
|
||||
await gcpEnsureGcloudCli();
|
||||
await gcpAuthenticate();
|
||||
// Deletion runs under a spinner — suppress interactive prompts
|
||||
const prevNonInteractive = process.env.SPAWN_NON_INTERACTIVE;
|
||||
process.env.SPAWN_NON_INTERACTIVE = "1";
|
||||
// resolveProject reads GCP_PROJECT directly — no fallback needed
|
||||
const resolveResult = await asyncTryCatch(() => gcpResolveProject());
|
||||
if (prevNonInteractive === undefined) {
|
||||
delete process.env.SPAWN_NON_INTERACTIVE;
|
||||
} else {
|
||||
process.env.SPAWN_NON_INTERACTIVE = prevNonInteractive;
|
||||
}
|
||||
if (!resolveResult.ok) {
|
||||
throw resolveResult.error;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1207,6 +1207,10 @@ export async function destroyInstance(name?: string): Promise<void> {
|
|||
throw new Error("No instance name");
|
||||
}
|
||||
|
||||
if (!_state.project) {
|
||||
throw new Error("No GCP project set — cannot determine which project to delete from");
|
||||
}
|
||||
|
||||
logStep(`Destroying GCP instance '${instanceName}'...`);
|
||||
const result = await gcloud([
|
||||
"compute",
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue