mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-09 16:00:03 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Paperclip provides CLI commands and guidance for operators and agents > - The `pnpm paperclipai` script can pass argument values through a shell > - Shell re-parsing can execute command substitutions inside quoted values > - This pull request routes guidance through inert-argv `npx paperclipai` commands and adds regression coverage > - The benefit is safer operator guidance across documentation and runtime hints ## Linked Issues or Issue Description This pull request fixes a command-injection-class defect in Paperclip CLI guidance. **What happened?** The `pnpm paperclipai <sub> --flag "$VALUE"` form can re-parse argument values through a shell. A command substitution inside a quoted value can execute on the host. **Expected behavior** Paperclip guidance must pass CLI values as inert argument values. Host-derived values must not appear in copyable commands. **Steps to reproduce** 1. Run a Paperclip guidance command that uses the `pnpm paperclipai` script. 2. Provide a quoted value that contains a command substitution. 3. Observe that the shell can evaluate the substitution before the CLI starts. 4. Compare the result with the `npx paperclipai` form. **Paperclip version or commit** `5670984b75d109950c968542a0111ebb6967f4da` **Deployment mode** All deployment modes that show or use the affected CLI guidance. **Installation method** Built from source and installed CLI guidance. **Agent adapter(s) involved** Not adapter-specific (core bug). **Database mode** Not database-related. **Access context** Both. **Additional context** The earlier merged PR [#11343](https://github.com/paperclipai/paperclip/pull/11343) used the unsafe `pnpm exec paperclipai` form. This fresh PR replaces that guidance with the safe `npx paperclipai` form. ## What Changed - Standardize documentation and runtime hints on `npx paperclipai`. - Remove the broken `pnpm exec paperclipai` guidance. - Use a static `<host>` placeholder in private-hostname guidance. - Add regression tests for unsafe forms, continued lines, static hosts, and offline guidance. ## Verification - `git diff --check origin/master...origin/fix/paperclipai-cli-npx-safe-invocation` passes. - The branch adds `server/src/__tests__/cli-invocation-safety.test.ts` and updates private-hostname tests. - CI must run the new tests, typecheck, lint, and build checks. - Local Vitest execution was not available because this worktree has no installed Vitest binary. ## Risks - The change affects operator and agent documentation text. - The runtime hints now show `<host>` instead of a request-derived host value. - No database schema or migration changes exist. - CI will detect any missed unsafe invocation or type error. ## Model Used OpenAI GPT-5, exact model ID `gpt-5`, with tool use and code-review assistance. The model used repository inspection, Git operations, and PR preparation. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] CI ran the test suites and they pass; local test execution was unavailable in this worktree - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I addressed all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
272 lines
7.5 KiB
TypeScript
272 lines
7.5 KiB
TypeScript
import { URL } from "node:url";
|
|
|
|
export class ApiRequestError extends Error {
|
|
status: number;
|
|
details?: unknown;
|
|
body?: unknown;
|
|
|
|
constructor(status: number, message: string, details?: unknown, body?: unknown) {
|
|
super(message);
|
|
this.status = status;
|
|
this.details = details;
|
|
this.body = body;
|
|
}
|
|
}
|
|
|
|
export class ApiConnectionError extends Error {
|
|
url: string;
|
|
method: string;
|
|
causeMessage?: string;
|
|
|
|
constructor(input: {
|
|
apiBase: string;
|
|
path: string;
|
|
method: string;
|
|
cause?: unknown;
|
|
}) {
|
|
const url = buildUrl(input.apiBase, input.path);
|
|
const causeMessage = formatConnectionCause(input.cause);
|
|
super(buildConnectionErrorMessage({ apiBase: input.apiBase, url, method: input.method, causeMessage }));
|
|
this.url = url;
|
|
this.method = input.method;
|
|
this.causeMessage = causeMessage;
|
|
}
|
|
}
|
|
|
|
interface RequestOptions {
|
|
ignoreNotFound?: boolean;
|
|
}
|
|
|
|
interface RecoverAuthInput {
|
|
path: string;
|
|
method: string;
|
|
error: ApiRequestError;
|
|
}
|
|
|
|
interface ApiClientOptions {
|
|
apiBase: string;
|
|
apiKey?: string;
|
|
runId?: string;
|
|
recoverAuth?: (input: RecoverAuthInput) => Promise<string | null>;
|
|
}
|
|
|
|
export class PaperclipApiClient {
|
|
readonly apiBase: string;
|
|
apiKey?: string;
|
|
readonly runId?: string;
|
|
readonly recoverAuth?: (input: RecoverAuthInput) => Promise<string | null>;
|
|
|
|
constructor(opts: ApiClientOptions) {
|
|
this.apiBase = opts.apiBase.replace(/\/+$/, "");
|
|
this.apiKey = opts.apiKey?.trim() || undefined;
|
|
this.runId = opts.runId?.trim() || undefined;
|
|
this.recoverAuth = opts.recoverAuth;
|
|
}
|
|
|
|
get<T>(path: string, opts?: RequestOptions): Promise<T | null> {
|
|
return this.request<T>(path, { method: "GET" }, opts);
|
|
}
|
|
|
|
post<T>(path: string, body?: unknown, opts?: RequestOptions): Promise<T | null> {
|
|
return this.request<T>(path, {
|
|
method: "POST",
|
|
body: body === undefined ? undefined : JSON.stringify(body),
|
|
}, opts);
|
|
}
|
|
|
|
patch<T>(path: string, body?: unknown, opts?: RequestOptions): Promise<T | null> {
|
|
return this.request<T>(path, {
|
|
method: "PATCH",
|
|
body: body === undefined ? undefined : JSON.stringify(body),
|
|
}, opts);
|
|
}
|
|
|
|
put<T>(path: string, body?: unknown, opts?: RequestOptions): Promise<T | null> {
|
|
return this.request<T>(path, {
|
|
method: "PUT",
|
|
body: body === undefined ? undefined : JSON.stringify(body),
|
|
}, opts);
|
|
}
|
|
|
|
/** Raw binary upload (e.g. one chunked import-transfer part); the body travels as-is. */
|
|
putRaw<T>(path: string, body: Uint8Array, opts?: RequestOptions): Promise<T | null> {
|
|
return this.request<T>(path, {
|
|
method: "PUT",
|
|
body: body as unknown as BodyInit,
|
|
headers: { "content-type": "application/octet-stream" },
|
|
}, opts);
|
|
}
|
|
|
|
delete<T>(path: string, opts?: RequestOptions): Promise<T | null> {
|
|
return this.request<T>(path, { method: "DELETE" }, opts);
|
|
}
|
|
|
|
setApiKey(apiKey: string | undefined) {
|
|
this.apiKey = apiKey?.trim() || undefined;
|
|
}
|
|
|
|
private async request<T>(
|
|
path: string,
|
|
init: RequestInit,
|
|
opts?: RequestOptions,
|
|
hasRetriedAuth = false,
|
|
): Promise<T | null> {
|
|
const url = buildUrl(this.apiBase, path);
|
|
const method = String(init.method ?? "GET").toUpperCase();
|
|
|
|
const headers: Record<string, string> = {
|
|
accept: "application/json",
|
|
...toStringRecord(init.headers),
|
|
};
|
|
|
|
if (init.body !== undefined) {
|
|
headers["content-type"] = headers["content-type"] ?? "application/json";
|
|
}
|
|
|
|
if (this.apiKey) {
|
|
headers.authorization = `Bearer ${this.apiKey}`;
|
|
}
|
|
|
|
if (this.runId) {
|
|
headers["x-paperclip-run-id"] = this.runId;
|
|
}
|
|
|
|
let response: Response;
|
|
try {
|
|
response = await fetch(url, {
|
|
...init,
|
|
headers,
|
|
});
|
|
} catch (error) {
|
|
throw new ApiConnectionError({
|
|
apiBase: this.apiBase,
|
|
path,
|
|
method,
|
|
cause: error,
|
|
});
|
|
}
|
|
|
|
if (opts?.ignoreNotFound && response.status === 404) {
|
|
return null;
|
|
}
|
|
|
|
if (!response.ok) {
|
|
const apiError = await toApiError(response);
|
|
if (!hasRetriedAuth && this.recoverAuth) {
|
|
const recoveredToken = await this.recoverAuth({
|
|
path,
|
|
method,
|
|
error: apiError,
|
|
});
|
|
if (recoveredToken) {
|
|
this.setApiKey(recoveredToken);
|
|
return this.request<T>(path, init, opts, true);
|
|
}
|
|
}
|
|
throw apiError;
|
|
}
|
|
|
|
if (response.status === 204) {
|
|
return null;
|
|
}
|
|
|
|
const text = await response.text();
|
|
if (!text.trim()) {
|
|
return null;
|
|
}
|
|
|
|
return safeParseJson(text) as T;
|
|
}
|
|
}
|
|
|
|
function buildUrl(apiBase: string, path: string): string {
|
|
const normalizedPath = path.startsWith("/") ? path : `/${path}`;
|
|
const [pathname, query] = normalizedPath.split("?");
|
|
const url = new URL(apiBase);
|
|
url.pathname = `${url.pathname.replace(/\/+$/, "")}${pathname}`;
|
|
if (query) url.search = query;
|
|
return url.toString();
|
|
}
|
|
|
|
function safeParseJson(text: string): unknown {
|
|
try {
|
|
return JSON.parse(text);
|
|
} catch {
|
|
return text;
|
|
}
|
|
}
|
|
|
|
async function toApiError(response: Response): Promise<ApiRequestError> {
|
|
const text = await response.text();
|
|
const parsed = safeParseJson(text);
|
|
|
|
if (typeof parsed === "object" && parsed !== null && !Array.isArray(parsed)) {
|
|
const body = parsed as Record<string, unknown>;
|
|
const message =
|
|
(typeof body.error === "string" && body.error.trim()) ||
|
|
(typeof body.message === "string" && body.message.trim()) ||
|
|
`Request failed with status ${response.status}`;
|
|
|
|
return new ApiRequestError(response.status, message, body.details, parsed);
|
|
}
|
|
|
|
return new ApiRequestError(response.status, `Request failed with status ${response.status}`, undefined, parsed);
|
|
}
|
|
|
|
function buildConnectionErrorMessage(input: {
|
|
apiBase: string;
|
|
url: string;
|
|
method: string;
|
|
causeMessage?: string;
|
|
}): string {
|
|
const healthUrl = buildHealthCheckUrl(input.url);
|
|
const lines = [
|
|
"Could not reach the Paperclip API.",
|
|
"",
|
|
`Request: ${input.method} ${input.url}`,
|
|
];
|
|
if (input.causeMessage) {
|
|
lines.push(`Cause: ${input.causeMessage}`);
|
|
}
|
|
lines.push(
|
|
"",
|
|
"This usually means the Paperclip server is not running, the configured URL is wrong, or the request is being blocked before it reaches Paperclip.",
|
|
"",
|
|
"Try:",
|
|
"- Start Paperclip with `pnpm dev` (from a source checkout) or `npx paperclipai run`.",
|
|
`- Verify the server is reachable with \`curl ${healthUrl}\`.`,
|
|
`- If Paperclip is running elsewhere, pass \`--api-base ${input.apiBase.replace(/\/+$/, "")}\` or set \`PAPERCLIP_API_URL\`.`,
|
|
);
|
|
return lines.join("\n");
|
|
}
|
|
|
|
function buildHealthCheckUrl(requestUrl: string): string {
|
|
const url = new URL(requestUrl);
|
|
url.pathname = `${url.pathname.replace(/\/+$/, "").replace(/\/api(?:\/.*)?$/, "")}/api/health`;
|
|
url.search = "";
|
|
url.hash = "";
|
|
return url.toString();
|
|
}
|
|
|
|
function formatConnectionCause(error: unknown): string | undefined {
|
|
if (!error) return undefined;
|
|
if (error instanceof Error) {
|
|
return error.message.trim() || error.name;
|
|
}
|
|
const message = String(error).trim();
|
|
return message || undefined;
|
|
}
|
|
|
|
function toStringRecord(headers: HeadersInit | undefined): Record<string, string> {
|
|
if (!headers) return {};
|
|
if (Array.isArray(headers)) {
|
|
return Object.fromEntries(headers.map(([key, value]) => [key, String(value)]));
|
|
}
|
|
if (headers instanceof Headers) {
|
|
return Object.fromEntries(headers.entries());
|
|
}
|
|
return Object.fromEntries(
|
|
Object.entries(headers).map(([key, value]) => [key, String(value)]),
|
|
);
|
|
}
|