Skip to content

Commit 231d001

Browse files
Patrick-ErichsenPatrick-Erichsen
committed
fix: harden github-backed skill installs
1 parent 4e6b120 commit 231d001

8 files changed

Lines changed: 67 additions & 20 deletions

File tree

‎src/agents/templates/HEARTBEAT.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
<!-- Heartbeat template; comments-only content prevents scheduled heartbeat API calls. -->
1+
# HEARTBEAT.md
22

3-
# Keep this file empty (or with only comments) to skip heartbeat API calls.
3+
# Keep this file empty to skip heartbeat API calls.
44

55
# Add tasks below when you want the agent to check something periodically.

‎src/agents/tools/pdf-tool.model-config.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ describe("resolvePdfModelConfigForTool", () => {
180180
} as OpenClawConfig;
181181

182182
expect(resolvePdfModelConfigForTool({ cfg, agentDir: TEST_AGENT_DIR })).toEqual({
183-
primary: "openai/gpt-5.4-mini",
183+
primary: "openai/gpt-5.5",
184184
fallbacks: ["minimax/MiniMax-M2.7", "minimax-portal/MiniMax-M2.7"],
185185
});
186186
});

‎src/cli/skills-cli.clawhub-install.e2e.test.ts‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { spawn } from "node:child_process";
22
import fs from "node:fs/promises";
3-
import { createServer, type IncomingMessage } from "node:http";
3+
import { createServer, type IncomingMessage, type ServerResponse } from "node:http";
44
import type { AddressInfo } from "node:net";
55
import os from "node:os";
66
import path from "node:path";
@@ -40,7 +40,7 @@ async function spawnOpenClaw(
4040
});
4141
}
4242

43-
async function buildGitHubSkillZip(commit: string): Promise<Buffer> {
43+
async function buildGitHubSkillZip(): Promise<Buffer> {
4444
const zip = new JSZip();
4545
zip.file("skills-main/skills/aiq-deploy/SKILL.md", "# AIQ Deploy\n");
4646
zip.file("skills-main/skills/aiq-deploy/skill-card.md", "# Card\n");
@@ -53,8 +53,8 @@ describe("openclaw skills install ClawHub GitHub-backed E2E", () => {
5353
const commit = "c".repeat(40);
5454
const telemetryBodies: unknown[] = [];
5555
const requestLog: string[] = [];
56-
const githubZipBytes = await buildGitHubSkillZip(commit);
57-
const server = createServer(async (req, res) => {
56+
const githubZipBytes = await buildGitHubSkillZip();
57+
async function handleRequest(req: IncomingMessage, res: ServerResponse): Promise<void> {
5858
const url = new URL(req.url ?? "/", "http://127.0.0.1");
5959
requestLog.push(`${req.method ?? "GET"} ${url.pathname}`);
6060

@@ -92,8 +92,18 @@ describe("openclaw skills install ClawHub GitHub-backed E2E", () => {
9292

9393
res.writeHead(404, { "Content-Type": "text/plain; charset=utf-8" });
9494
res.end("not found");
95+
}
96+
const server = createServer((req, res) => {
97+
void handleRequest(req, res).catch((error: unknown) => {
98+
res.writeHead(500, { "Content-Type": "text/plain; charset=utf-8" });
99+
res.end(error instanceof Error ? error.message : String(error));
100+
});
101+
});
102+
await new Promise<void>((resolve) => {
103+
server.listen(0, "127.0.0.1", () => {
104+
resolve();
105+
});
95106
});
96-
await new Promise<void>((resolve) => server.listen(0, "127.0.0.1", resolve));
97107

98108
const registry = `http://127.0.0.1:${(server.address() as AddressInfo).port}`;
99109
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-clawhub-cli-e2e-"));
@@ -134,7 +144,11 @@ describe("openclaw skills install ClawHub GitHub-backed E2E", () => {
134144
],
135145
});
136146
} finally {
137-
await new Promise<void>((resolve) => server.close(() => resolve()));
147+
await new Promise<void>((resolve) => {
148+
server.close(() => {
149+
resolve();
150+
});
151+
});
138152
await fs.rm(stateDir, { recursive: true, force: true });
139153
}
140154
}, 30_000);

‎src/crestodian/rescue-message.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import fs from "node:fs/promises";
22
import os from "node:os";
33
import path from "node:path";
4-
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
4+
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
55
import type { CommandContext } from "../auto-reply/reply/commands-types.js";
66
import type { OpenClawConfig } from "../config/types.openclaw.js";
77
import type { RuntimeEnv } from "../runtime.js";

‎src/infra/clawhub.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { withTempDir } from "../test-helpers/temp-dir.js";
88
import {
99
downloadClawHubPackageArchive,
1010
downloadClawHubSkillArchive,
11+
downloadClawHubSkillArchiveUrl,
1112
fetchClawHubSkillCard,
1213
fetchClawHubSkillSecurityVerdicts,
1314
fetchClawHubPackageArtifact,
@@ -855,4 +856,33 @@ describe("clawhub helpers", () => {
855856
await expectPathMissing(archiveDir);
856857
}
857858
});
859+
860+
it("does not send ambient ClawHub auth tokens to off-registry resolver archive URLs", async () => {
861+
process.env.OPENCLAW_CLAWHUB_TOKEN = "env-token-123";
862+
let requestedUrl = "";
863+
let requestedInit: RequestInit | undefined;
864+
865+
const archive = await downloadClawHubSkillArchiveUrl({
866+
baseUrl: "https://clawhub.ai",
867+
url: "https://codeload.github.com/NVIDIA/skills/zip/abcdef",
868+
fetchImpl: async (input, init) => {
869+
requestedUrl = input instanceof Request ? input.url : String(input);
870+
requestedInit = init;
871+
return new Response(new Uint8Array([7, 8, 9]), {
872+
status: 200,
873+
headers: { "content-type": "application/zip" },
874+
});
875+
},
876+
});
877+
878+
try {
879+
expect(requestedUrl).toBe("https://codeload.github.com/NVIDIA/skills/zip/abcdef");
880+
expect(new Headers(requestedInit?.headers).get("Authorization")).toBeNull();
881+
await expect(fs.readFile(archive.archivePath)).resolves.toEqual(Buffer.from([7, 8, 9]));
882+
} finally {
883+
const archiveDir = path.dirname(archive.archivePath);
884+
await archive.cleanup();
885+
await expectPathMissing(archiveDir);
886+
}
887+
});
858888
});

‎src/infra/clawhub.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1364,12 +1364,17 @@ export async function downloadClawHubSkillArchiveUrl(params: {
13641364
timeoutMs?: number;
13651365
fetchImpl?: FetchLike;
13661366
}): Promise<ClawHubDownloadResult> {
1367+
const explicitToken = normalizeOptionalString(params.token);
1368+
const requestUrl = new URL(params.url, `${normalizeBaseUrl(params.baseUrl)}/`);
1369+
const registryOrigin = new URL(`${normalizeBaseUrl(params.baseUrl)}/`).origin;
1370+
const skipAuth = explicitToken == null && requestUrl.origin !== registryOrigin;
13671371
const { response, url, hasToken } = await clawhubRequest({
13681372
baseUrl: params.baseUrl,
13691373
url: params.url,
1370-
token: params.token,
1374+
token: explicitToken,
13711375
timeoutMs: params.timeoutMs,
13721376
fetchImpl: params.fetchImpl,
1377+
skipAuth,
13731378
});
13741379
if (!response.ok) {
13751380
throw await buildClawHubError(response, url, hasToken);

‎src/skills/lifecycle/clawhub.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -892,9 +892,7 @@ async function performClawHubSkillInstall(
892892
let version!: string;
893893
let detail: ClawHubSkillDetail | undefined;
894894
let latestResolution: Extract<ClawHubSkillInstallResolutionResponse, { ok: true }> | undefined;
895-
let install:
896-
| Awaited<ReturnType<typeof installArchiveResolution>>
897-
| Awaited<ReturnType<typeof installGitHubResolution>>;
895+
let install: Awaited<ReturnType<typeof installArchiveResolution>>;
898896

899897
const archive = params.version
900898
? await (async () => {

‎src/tasks/task-flow-registry.store.test.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ describe("task-flow-registry store runtime", () => {
150150
});
151151

152152
it("rejects corrupt persisted flow rows during sqlite restore", async () => {
153-
await withFlowRegistryTempDir(async (root) => {
153+
await withFlowRegistryTempDir(async () => {
154154
resetTaskFlowRegistryForTests();
155155

156156
const created = createManagedTaskFlow({
@@ -174,7 +174,7 @@ describe("task-flow-registry store runtime", () => {
174174
});
175175

176176
it("drops invalid requester origins during sqlite restore", async () => {
177-
await withFlowRegistryTempDir(async (root) => {
177+
await withFlowRegistryTempDir(async () => {
178178
resetTaskFlowRegistryForTests();
179179

180180
const created = createManagedTaskFlow({
@@ -203,7 +203,7 @@ describe("task-flow-registry store runtime", () => {
203203
});
204204

205205
it("restores persisted wait-state, revision, and cancel intent from sqlite", async () => {
206-
await withFlowRegistryTempDir(async (root) => {
206+
await withFlowRegistryTempDir(async () => {
207207
resetTaskFlowRegistryForTests();
208208

209209
const created = createManagedTaskFlow({
@@ -248,7 +248,7 @@ describe("task-flow-registry store runtime", () => {
248248
});
249249

250250
it("round-trips explicit json null through sqlite", async () => {
251-
await withFlowRegistryTempDir(async (root) => {
251+
await withFlowRegistryTempDir(async () => {
252252
resetTaskFlowRegistryForTests();
253253

254254
const created = createManagedTaskFlow({
@@ -269,7 +269,7 @@ describe("task-flow-registry store runtime", () => {
269269
});
270270

271271
it("prunes large sqlite snapshots without binding every flow id at once", async () => {
272-
await withFlowRegistryTempDir(async (root) => {
272+
await withFlowRegistryTempDir(async () => {
273273
resetTaskFlowRegistryForTests();
274274

275275
const flows = new Map<string, TaskFlowRecord>();
@@ -299,7 +299,7 @@ describe("task-flow-registry store runtime", () => {
299299
if (process.platform === "win32") {
300300
return;
301301
}
302-
await withFlowRegistryTempDir(async (root) => {
302+
await withFlowRegistryTempDir(async () => {
303303
resetTaskFlowRegistryForTests();
304304

305305
createManagedTaskFlow({

0 commit comments

Comments
 (0)