mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 20:46:28 +01:00
fix(server): validate CLI auth challenge IDs (#14095)
Reject malformed CLI challenge UUIDs before database access while preserving secret and authentication precedence. Document the HTTP responses and pin the supervisor test fixture to the CI-selected Node executable. Validated by 21 focused tests, root typecheck/build, and full CI. Mac broad-suite baseline limitations are documented in the PR. Greptile 5/5. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
1e3a148f46
commit
d3e0f0a238
5 files changed
+87
-17
No files matched your search
@@ -932,6 +932,12 @@ npx paperclipai auth revoke-current
|
||||
|
||||
`--token <challenge-secret>` is still supported for compatibility, but `--token-env` avoids putting challenge secrets in shell history or process arguments.
|
||||
|
||||
Use the challenge UUID returned by `auth challenge create` for get, approve, and
|
||||
cancel. With the required secret and approval authentication present, malformed
|
||||
IDs return HTTP 400 before database access. A status request without a secret
|
||||
returns HTTP 404. Unknown challenges or incorrect challenge secrets still return
|
||||
HTTP 404. Approval requires board authentication, checked before ID validation.
|
||||
|
||||
## Instance Settings Commands
|
||||
|
||||
```sh
|
||||
|
||||
@@ -104,7 +104,7 @@ describe.sequential("cli auth routes", () => {
|
||||
it.sequential("creates a CLI auth challenge with approval metadata", async () => {
|
||||
mockBoardAuthService.createCliAuthChallenge.mockResolvedValue({
|
||||
challenge: {
|
||||
id: "challenge-1",
|
||||
id: "12345678-1234-4123-8123-123456789abc",
|
||||
expiresAt: new Date("2026-03-23T13:00:00.000Z"),
|
||||
},
|
||||
challengeSecret: "pcp_cli_auth_secret",
|
||||
@@ -122,14 +122,14 @@ describe.sequential("cli auth routes", () => {
|
||||
|
||||
expect(res.status, res.text || JSON.stringify(res.body)).toBe(201);
|
||||
expect(res.body).toMatchObject({
|
||||
id: "challenge-1",
|
||||
id: "12345678-1234-4123-8123-123456789abc",
|
||||
token: "pcp_cli_auth_secret",
|
||||
approvalPath: "/cli-auth/challenge-1?token=pcp_cli_auth_secret",
|
||||
pollPath: "/cli-auth/challenges/challenge-1",
|
||||
approvalPath: "/cli-auth/12345678-1234-4123-8123-123456789abc?token=pcp_cli_auth_secret",
|
||||
pollPath: "/cli-auth/challenges/12345678-1234-4123-8123-123456789abc",
|
||||
expiresAt: "2026-03-23T13:00:00.000Z",
|
||||
});
|
||||
expect(res.body.boardApiToken).toBe("pcp_board_token");
|
||||
expect(res.body.approvalUrl).toContain("/cli-auth/challenge-1?token=pcp_cli_auth_secret");
|
||||
expect(res.body.approvalUrl).toContain("/cli-auth/12345678-1234-4123-8123-123456789abc?token=pcp_cli_auth_secret");
|
||||
});
|
||||
|
||||
it.sequential("rejects anonymous access to generic skill documents", async () => {
|
||||
@@ -176,7 +176,7 @@ describe.sequential("cli auth routes", () => {
|
||||
|
||||
it.sequential("marks challenge status as requiring sign-in for anonymous viewers", async () => {
|
||||
mockBoardAuthService.describeCliAuthChallenge.mockResolvedValue({
|
||||
id: "challenge-1",
|
||||
id: "12345678-1234-4123-8123-123456789abc",
|
||||
status: "pending",
|
||||
command: "paperclipai company import",
|
||||
clientName: "paperclipai cli",
|
||||
@@ -190,18 +190,67 @@ describe.sequential("cli auth routes", () => {
|
||||
});
|
||||
|
||||
const app = await createApp({ type: "none", source: "none" });
|
||||
const res = await request(app).get("/api/cli-auth/challenges/challenge-1?token=pcp_cli_auth_secret");
|
||||
const res = await request(app).get("/api/cli-auth/challenges/12345678-1234-4123-8123-123456789abc?token=pcp_cli_auth_secret");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.requiresSignIn).toBe(true);
|
||||
expect(res.body.canApprove).toBe(false);
|
||||
});
|
||||
|
||||
it.each(["PASTE_ID_HERE", "not-a-uuid"])("rejects malformed challenge ID %s before calling the service", async (id) => {
|
||||
const app = await createApp({ type: "board", userId: "user-1", source: "session" });
|
||||
const responses = [
|
||||
await request(app).get(`/api/cli-auth/challenges/${id}?token=pcp_cli_auth_secret`),
|
||||
await request(app).post(`/api/cli-auth/challenges/${id}/approve`).send({ token: "pcp_cli_auth_secret" }),
|
||||
await request(app).post(`/api/cli-auth/challenges/${id}/cancel`).send({ token: "pcp_cli_auth_secret" }),
|
||||
];
|
||||
for (const response of responses) {
|
||||
expect(response.status).toBe(400);
|
||||
expect(response.body.error).toBe("Invalid CLI auth challenge ID");
|
||||
}
|
||||
expect(mockBoardAuthService.describeCliAuthChallenge).not.toHaveBeenCalled();
|
||||
expect(mockBoardAuthService.approveCliAuthChallenge).not.toHaveBeenCalled();
|
||||
expect(mockBoardAuthService.cancelCliAuthChallenge).not.toHaveBeenCalled();
|
||||
expect(mockLogActivity).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("requires authentication before approving even a malformed challenge ID", async () => {
|
||||
const app = await createApp({ type: "none", source: "none" });
|
||||
const res = await request(app)
|
||||
.post("/api/cli-auth/challenges/PASTE_ID_HERE/approve")
|
||||
.send({ token: "pcp_cli_auth_secret" });
|
||||
expect(res.status).toBe(401);
|
||||
expect(mockBoardAuthService.approveCliAuthChallenge).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("preserves missing-secret and unknown-challenge responses", async () => {
|
||||
const app = await createApp({ type: "none", source: "none" });
|
||||
const path = "/api/cli-auth/challenges/12345678-1234-4123-8123-123456789abc";
|
||||
const missing = await request(app).get(path);
|
||||
expect(missing.status).toBe(404);
|
||||
expect(mockBoardAuthService.describeCliAuthChallenge).not.toHaveBeenCalled();
|
||||
mockBoardAuthService.describeCliAuthChallenge.mockResolvedValue(null);
|
||||
const unknown = await request(app).get(`${path}?token=wrong-secret`);
|
||||
expect(unknown.status).toBe(404);
|
||||
});
|
||||
|
||||
it("cancels a valid challenge and preserves path whitespace normalization", async () => {
|
||||
mockBoardAuthService.cancelCliAuthChallenge.mockResolvedValue({ status: "cancelled" });
|
||||
const app = await createApp({ type: "none", source: "none" });
|
||||
const id = "12345678-1234-4123-8123-123456789ABC";
|
||||
const res = await request(app)
|
||||
.post(`/api/cli-auth/challenges/${encodeURIComponent(` ${id} `)}/cancel`)
|
||||
.send({ token: "pcp_cli_auth_secret" });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body).toEqual({ status: "cancelled", cancelled: true });
|
||||
expect(mockBoardAuthService.cancelCliAuthChallenge).toHaveBeenCalledWith(id, "pcp_cli_auth_secret");
|
||||
});
|
||||
|
||||
it.sequential("approves a CLI auth challenge for a signed-in board user", async () => {
|
||||
mockBoardAuthService.approveCliAuthChallenge.mockResolvedValue({
|
||||
status: "approved",
|
||||
challenge: {
|
||||
id: "challenge-1",
|
||||
id: "12345678-1234-4123-8123-123456789abc",
|
||||
boardApiKeyId: "board-key-1",
|
||||
requestedAccess: "board",
|
||||
requestedCompanyId: "company-1",
|
||||
@@ -223,12 +272,12 @@ describe.sequential("cli auth routes", () => {
|
||||
companyIds: ["company-1"],
|
||||
});
|
||||
const res = await request(app)
|
||||
.post("/api/cli-auth/challenges/challenge-1/approve")
|
||||
.post("/api/cli-auth/challenges/12345678-1234-4123-8123-123456789abc/approve")
|
||||
.send({ token: "pcp_cli_auth_secret" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(mockBoardAuthService.approveCliAuthChallenge).toHaveBeenCalledWith(
|
||||
"challenge-1",
|
||||
"12345678-1234-4123-8123-123456789abc",
|
||||
"pcp_cli_auth_secret",
|
||||
"user-1",
|
||||
);
|
||||
@@ -246,7 +295,7 @@ describe.sequential("cli auth routes", () => {
|
||||
mockBoardAuthService.approveCliAuthChallenge.mockResolvedValue({
|
||||
status: "approved",
|
||||
challenge: {
|
||||
id: "challenge-2",
|
||||
id: "12345678-1234-4123-8123-123456789def",
|
||||
boardApiKeyId: "board-key-2",
|
||||
requestedAccess: "instance_admin_required",
|
||||
requestedCompanyId: null,
|
||||
@@ -263,7 +312,7 @@ describe.sequential("cli auth routes", () => {
|
||||
companyIds: [],
|
||||
});
|
||||
const res = await request(app)
|
||||
.post("/api/cli-auth/challenges/challenge-2/approve")
|
||||
.post("/api/cli-auth/challenges/12345678-1234-4123-8123-123456789def/approve")
|
||||
.send({ token: "pcp_cli_auth_secret" });
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
@@ -31,6 +31,13 @@ describe("local service supervision", () => {
|
||||
process.env.PAPERCLIP_HOME = paperclipHome;
|
||||
process.env.PAPERCLIP_INSTANCE_ID = `service-stdio-${randomUUID()}`;
|
||||
|
||||
// The managed service uses a login shell, whose PATH can select a different
|
||||
// Node installation from the one CI configured for this test process.
|
||||
// Git Bash accepts Windows drive paths with forward slashes.
|
||||
const nodeExecutable = process.platform === "win32"
|
||||
? process.execPath.replace(/\\/g, "/")
|
||||
: process.execPath;
|
||||
const nodeCommand = "'" + nodeExecutable.replace(/'/g, "'\\''") + "'";
|
||||
let registryRecord: Awaited<ReturnType<typeof listLocalServiceRegistryRecords>>[number] | null = null;
|
||||
try {
|
||||
const [service] = await startRuntimeServicesForWorkspaceControl({
|
||||
@@ -54,7 +61,7 @@ describe("local service supervision", () => {
|
||||
workspaceRuntime: {
|
||||
services: [{
|
||||
name: "web",
|
||||
command: "command -v node; node --version; node -e \"const http=require('node:http'); console.log('fixture starting', process.execPath, process.env.PORT); process.on('SIGTERM',()=>{}); http.createServer((req,res)=>{ process.stdout.write('request '+req.url+'\\\\n',(error)=>{ if (!error) res.end('ok'); }); }).listen(Number(process.env.PORT), '127.0.0.1',()=>console.log('fixture listening', process.env.PORT))\"",
|
||||
command: nodeCommand + " -e \"const http=require('node:http'); console.log('fixture starting', process.execPath, process.env.PORT); process.on('SIGTERM',()=>{}); http.createServer((req,res)=>{ process.stdout.write('request '+req.url+'\\\\n',(error)=>{ if (!error) res.end('ok'); }); }).listen(Number(process.env.PORT), '127.0.0.1',()=>console.log('fixture listening', process.env.PORT))\"",
|
||||
port: { type: "auto" },
|
||||
readiness: {
|
||||
type: "http",
|
||||
@@ -82,6 +89,7 @@ describe("local service supervision", () => {
|
||||
await expect(fetch(`${service!.url}/after-restart`)).resolves.toMatchObject({ ok: true });
|
||||
const log = await fs.readFile(resolveLocalServiceLogPath(registryRecord!.serviceKey), "utf8");
|
||||
expect(log).toContain("request /after-restart");
|
||||
expect(log).toContain(process.execPath);
|
||||
|
||||
await terminateLocalService(registryRecord!);
|
||||
expect(await readLocalServicePortOwner(service!.port!)).toBeNull();
|
||||
|
||||
@@ -2792,6 +2792,7 @@ export function accessRoutes(
|
||||
const token =
|
||||
typeof req.query.token === "string" ? req.query.token.trim() : "";
|
||||
if (!id || !token) throw notFound("CLI auth challenge not found");
|
||||
if (!isUuidLike(id)) throw badRequest("Invalid CLI auth challenge ID");
|
||||
const challenge = await boardAuth.describeCliAuthChallenge(id, token);
|
||||
if (!challenge) throw notFound("CLI auth challenge not found");
|
||||
|
||||
@@ -2824,6 +2825,7 @@ export function accessRoutes(
|
||||
) {
|
||||
throw unauthorized("Sign in before approving CLI access");
|
||||
}
|
||||
if (!isUuidLike(id)) throw badRequest("Invalid CLI auth challenge ID");
|
||||
|
||||
const userId = req.actor.userId ?? "local-board";
|
||||
const approved = await boardAuth.approveCliAuthChallenge(
|
||||
@@ -2871,6 +2873,7 @@ export function accessRoutes(
|
||||
validate(resolveCliAuthChallengeSchema),
|
||||
async (req, res) => {
|
||||
const id = (req.params.id as string).trim();
|
||||
if (!isUuidLike(id)) throw badRequest("Invalid CLI auth challenge ID");
|
||||
const cancelled = await boardAuth.cancelCliAuthChallenge(id, req.body.token);
|
||||
res.json({
|
||||
status: cancelled.status,
|
||||
|
||||
@@ -636,6 +636,10 @@ const heartbeatRunIdParamSchema = z.string()
|
||||
.regex(/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[1-5][0-9a-fA-F]{3}-[89aAbB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$/)
|
||||
.describe("Heartbeat run UUID; malformed values return 400");
|
||||
|
||||
const cliAuthChallengeIdParamSchema = z.string().trim()
|
||||
.regex(/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[1-5][0-9a-fA-F]{3}-[89aAbB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$/)
|
||||
.describe("CLI auth challenge UUID; malformed values return 400");
|
||||
|
||||
const ErrorSchema = registry.register("Error", z.object({ error: z.string() }));
|
||||
|
||||
const responses = {
|
||||
@@ -6468,7 +6472,7 @@ registry.registerPath({
|
||||
tags: ["access"],
|
||||
summary: "Approve a CLI auth challenge",
|
||||
request: {
|
||||
params: z.object({ id: z.string() }),
|
||||
params: z.object({ id: cliAuthChallengeIdParamSchema }),
|
||||
body: jsonBody(resolveCliAuthChallengeSchema),
|
||||
},
|
||||
responses: {
|
||||
@@ -6485,7 +6489,7 @@ registry.registerPath({
|
||||
tags: ["access"],
|
||||
summary: "Cancel a CLI auth challenge",
|
||||
request: {
|
||||
params: z.object({ id: z.string() }),
|
||||
params: z.object({ id: cliAuthChallengeIdParamSchema }),
|
||||
body: jsonBody(resolveCliAuthChallengeSchema),
|
||||
},
|
||||
responses: { 200: r.ok(), 400: r.badRequest, 404: r.notFound },
|
||||
@@ -9201,8 +9205,8 @@ registry.registerPath({
|
||||
path: "/api/cli-auth/challenges/{id}",
|
||||
tags: ["access"],
|
||||
summary: "Get a CLI auth challenge",
|
||||
request: { params: z.object({ id: z.string() }) },
|
||||
responses: { 200: r.ok(), 404: r.notFound },
|
||||
request: { params: z.object({ id: cliAuthChallengeIdParamSchema }) },
|
||||
responses: { 200: r.ok(), 400: r.badRequest, 404: r.notFound },
|
||||
});
|
||||
|
||||
// ─── Invite onboarding ────────────────────────────────────────────────────────
|
||||
|
||||
Reference in new issue
Block a user