mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-14 01:50:39 +09:00
fix(mcp): tighten api request validation
This commit is contained in:
parent
85ca675311
commit
669e5c87cc
4 changed files with 171 additions and 2 deletions
|
|
@ -63,6 +63,7 @@ Write tools:
|
|||
- `paperclipAddComment`
|
||||
- `paperclipUpsertIssueDocument`
|
||||
- `paperclipRestoreIssueDocumentRevision`
|
||||
- `paperclipCreateApproval`
|
||||
- `paperclipLinkIssueApproval`
|
||||
- `paperclipUnlinkIssueApproval`
|
||||
- `paperclipApprovalDecision`
|
||||
|
|
|
|||
|
|
@ -107,6 +107,32 @@ describe("paperclip MCP tools", () => {
|
|||
});
|
||||
});
|
||||
|
||||
it("creates approvals with the expected company-scoped payload", async () => {
|
||||
const fetchMock = vi.fn().mockResolvedValue(
|
||||
mockJsonResponse({ id: "approval-1" }),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const tool = getTool("paperclipCreateApproval");
|
||||
await tool.execute({
|
||||
type: "hire_agent",
|
||||
payload: { branch: "pap-1167" },
|
||||
issueIds: ["44444444-4444-4444-4444-444444444444"],
|
||||
});
|
||||
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
const [url, init] = fetchMock.mock.calls[0] as [string, RequestInit];
|
||||
expect(String(url)).toBe(
|
||||
"http://localhost:3100/api/companies/11111111-1111-1111-1111-111111111111/approvals",
|
||||
);
|
||||
expect(init.method).toBe("POST");
|
||||
expect(JSON.parse(String(init.body))).toEqual({
|
||||
type: "hire_agent",
|
||||
payload: { branch: "pap-1167" },
|
||||
issueIds: ["44444444-4444-4444-4444-444444444444"],
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects invalid generic request paths", async () => {
|
||||
vi.stubGlobal("fetch", vi.fn());
|
||||
|
||||
|
|
@ -118,4 +144,16 @@ describe("paperclip MCP tools", () => {
|
|||
|
||||
expect(response.content[0]?.text).toContain("path must start with /");
|
||||
});
|
||||
|
||||
it("rejects generic request paths that escape /api", async () => {
|
||||
vi.stubGlobal("fetch", vi.fn());
|
||||
|
||||
const tool = getTool("paperclipApiRequest");
|
||||
const response = await tool.execute({
|
||||
method: "GET",
|
||||
path: "/../../secret",
|
||||
});
|
||||
|
||||
expect(response.content[0]?.text).toContain("must not contain '..'");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -415,8 +415,8 @@ export function createToolDefinitions(client: PaperclipApiClient): ToolDefinitio
|
|||
"Make a JSON request to an existing Paperclip /api endpoint for unsupported operations",
|
||||
apiRequestSchema,
|
||||
async ({ method, path, jsonBody }) => {
|
||||
if (!path.startsWith("/")) {
|
||||
throw new Error("path must start with / and be relative to /api");
|
||||
if (!path.startsWith("/") || path.includes("..")) {
|
||||
throw new Error("path must start with / and be relative to /api, and must not contain '..'");
|
||||
}
|
||||
return client.requestJson(method, path, {
|
||||
body: parseOptionalJson(jsonBody),
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue