mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-06 23:20:53 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent owners configure the harness and computer used for that work. > - A legacy runner upgrade must first qualify an unverified sandbox. > - The upgrade called an instance-admin probe, so a company owner with agent configuration permission received HTTP 403. > - This pull request qualifies the saved computer through the agent's existing configuration authorization. > - Company owners can complete the upgrade while general instance environment management stays restricted. ## Linked Issues or Issue Description Refs #17361. Related: #17122. **What happened?** Upgrade runner displayed for an existing Claude agent on a Daytona Paperclip Computer. Clicking it returned “Instance admin access required” for a cloud company owner. The agent remained on the legacy runner. **Expected behavior** A caller permitted to update the agent can qualify that agent's saved computer and upgrade. Instance-admin access is not required for this agent-scoped operation. **Steps to reproduce** 1. Sign in as a cloud company owner with `agents:configure` and no instance-admin role. 2. Open an existing legacy Claude agent on an unqualified Daytona snapshot. 3. Select Upgrade runner. ## What Changed - Add `POST /api/agents/{id}/qualify-runner` with the existing `agent_config:update` guard. - Reuse the server's effective environment resolver. The submitted environment ID only detects stale state; it cannot select a different computer. - Preserve company binding checks, check binding again at lease acquisition, and record agent-scoped audit activity. - Return qualification status and the lease receipt without instance-level provider metadata or raw provider errors. - Use the new endpoint for the existing-agent upgrade. Keep the generic environment probe restricted to instance admins. - Disable Upgrade runner when an unsaved computer change resolves to a different target, with a save-first explanation. Other draft settings still accompany an upgrade. - Add real-database authorization regression tests and update the API contract and access maps. ## Verification - Red: the existing UI called the instance-wide probe and failed the new agent-scoped call assertions in eight upgrade cases. - Green: 349 related route, provider qualification, runner selection, and UI tests passed. - Green: 12 authorization cases passed with real PostgreSQL, real HTTP routes, real saved role grants, and a simulated provider boot. They cover the cloud owner, removed grants, foreign agent/computer, inherited targets, stale target, invalid body, archived/local target, redacted failure, audit scope, and unchanged agent settings. - Review regression: two cases first reproduced the enabled upgrade button after an unsaved computer change; both now pass, along with all 168 related UI tests. Qualified/unqualified computers and restoring the saved selection are covered. - All 26 OpenAPI cases passed; the 12 real-database authorization cases also passed after rebase. - The host exhausted PostgreSQL semaphores. For the 12 cases, a temporary local-only setup used disposable PostgreSQL 18 in Docker instead of the embedded database launcher. The committed tests use the normal embedded database helper. No authorization decisions were mocked. - `pnpm -r typecheck`, `pnpm build`, `pnpm check:token-gates`, map validation, module-boundary validation, and `git diff --check` passed. - `pnpm test:run` was attempted but did not establish a full local pass: the host could not start embedded PostgreSQL clusters. All current-head CI gates passed, including the full suite in CI. The commitperclip review workflow timed out before reporting any executed steps on its first attempt and passed on one retry. - Fresh staging acceptance remains pending deployment. Verify persisted native configuration and a successful runtime test on the existing Daytona computer after merge. Earlier live Daytona proof from #17361 is separate evidence. ## Risks - Qualification can start a fresh, temporary sandbox. It uses the existing probe lifecycle and cleanup contract. - The new path must remain scoped to the saved agent and its company. Denial and redaction tests cover that boundary. General instance probe authorization is unchanged. - No migration, role change, credential change, or automatic agent upgrade is included. ## Model Used - OpenAI GPT-6 through Codex, with reasoning, code execution, and browser tools. The exact deployed model variant and context-window size were not exposed in this session. ## 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] I have run tests locally and they pass - [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 will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
124 lines
8.4 KiB
TypeScript
124 lines
8.4 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import express from "express";
|
|
import request from "supertest";
|
|
import { eq } from "drizzle-orm";
|
|
import { afterAll, beforeAll, beforeEach, describe, expect, it, vi } from "vitest";
|
|
import { agents, authUsers, companies, companyMemberships, createDb, environments, principalPermissionGrants, activityLog, instanceSettings, builtInManagedResources, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS } from "@paperclipai/db";
|
|
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
|
import { grantsForHumanRole } from "../services/company-member-roles.js";
|
|
import { agentRoutes } from "../routes/agents.js";
|
|
import { environmentRoutes } from "../routes/environments.js";
|
|
import { errorHandler } from "../middleware/error-handler.js";
|
|
|
|
const probe = vi.hoisted(() => vi.fn());
|
|
vi.mock("../services/environment-probe.js", async original => ({
|
|
...await original<object>(), probeEnvironment: probe,
|
|
}));
|
|
|
|
const support = await getEmbeddedPostgresTestSupport();
|
|
describe.skipIf(!support.supported)("agent-scoped runner qualification", () => {
|
|
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
let companyId: string;
|
|
let agentId: string;
|
|
let environmentId: string;
|
|
let userId: string;
|
|
let app: express.Express;
|
|
|
|
beforeAll(async () => {
|
|
tempDb = await startEmbeddedPostgresTestDatabase("runner-qualification-");
|
|
db = createDb(tempDb.connectionString);
|
|
}, EMBEDDED_POSTGRES_TEST_TIMEOUT_MS);
|
|
afterAll(async () => { await tempDb?.cleanup(); });
|
|
beforeEach(async () => {
|
|
await db.delete(instanceSettings);
|
|
await db.delete(environments);
|
|
probe.mockReset().mockResolvedValue({ ok: true, driver: "sandbox", summary: "Connected",
|
|
details: { platformQualificationRecorded: true, leaseId: "probe-lease", metadata: { private: "provider-secret" }, providerLeaseId: "private-id" } });
|
|
companyId = randomUUID(); agentId = randomUUID(); environmentId = randomUUID(); userId = randomUUID();
|
|
await db.insert(companies).values({ id: companyId, name: "Qualification", issuePrefix: `Q${companyId.slice(0, 7)}` });
|
|
await db.insert(authUsers).values({ id: userId, name: "Company owner", email: `${userId}@example.test`, emailVerified: true, createdAt: new Date(), updatedAt: new Date() });
|
|
await db.insert(companyMemberships).values({ companyId, principalType: "user", principalId: userId, membershipRole: "owner", status: "active" });
|
|
await db.insert(principalPermissionGrants).values(grantsForHumanRole("owner").map(grant => ({ ...grant, companyId, principalType: "user" as const, principalId: userId })));
|
|
await db.insert(environments).values({ id: environmentId, name: `Paperclip Computer ${environmentId}`, driver: "sandbox", status: "active", config: { provider: "daytona", snapshot: "managed-snapshot" } });
|
|
await db.insert(agents).values({ id: agentId, companyId, name: "Amy fixture", adapterType: "claude_local", defaultEnvironmentId: environmentId,
|
|
adapterConfig: { model: "claude-opus-5-5", effort: "high", maxTurnsPerRun: 1000, dangerouslySkipPermissions: true } });
|
|
app = express(); app.use(express.json());
|
|
app.use((req, _res, next) => {
|
|
req.actor = { type: "board", userId, companyIds: [companyId], source: "cloud_tenant", isInstanceAdmin: false,
|
|
memberships: [{ companyId, membershipRole: "owner", status: "active" }] };
|
|
next();
|
|
});
|
|
app.use("/api", agentRoutes(db));
|
|
app.use("/api", environmentRoutes(db));
|
|
app.use(errorHandler);
|
|
});
|
|
const qualify = () => request(app).post(`/api/agents/${agentId}/qualify-runner`).send({ expectedEnvironmentId: environmentId });
|
|
|
|
it("allows the company owner without instance admin and keeps the generic probe restricted", async () => {
|
|
const before = await db.select().from(agents).where(eq(agents.id, agentId));
|
|
const res = await qualify();
|
|
expect(res.status).toBe(200);
|
|
expect(res.body).toMatchObject({ ok: true, details: { platformQualificationRecorded: true, leaseId: "probe-lease" } });
|
|
expect(JSON.stringify(res.body)).not.toMatch(/provider-secret|private-id/);
|
|
expect(probe).toHaveBeenCalledWith(db, expect.objectContaining({ id: environmentId }), expect.objectContaining({ companyId, assertCompanyBinding: true, acquireSandboxRuntimeLease: true, applyCustomImageTemplate: true }));
|
|
expect(await db.select().from(agents).where(eq(agents.id, agentId))).toEqual(before);
|
|
const audit = await db.select().from(activityLog).where(eq(activityLog.entityId, agentId));
|
|
expect(audit).toEqual([expect.objectContaining({ companyId, actorId: userId, action: "agent.runner_qualification_tested" })]);
|
|
const generic = await request(app).post(`/api/environments/${environmentId}/probe?companyId=${companyId}`).send({});
|
|
expect(generic.status).toBe(403);
|
|
expect(probe).toHaveBeenCalledTimes(1);
|
|
});
|
|
it("rejects an owner whose configure grant is absent before any provider work", async () => {
|
|
await db.delete(principalPermissionGrants).where(eq(principalPermissionGrants.principalId, userId));
|
|
expect((await qualify()).status).toBe(403);
|
|
expect(probe).not.toHaveBeenCalled();
|
|
});
|
|
it("hides a foreign agent before any provider work", async () => {
|
|
const other = randomUUID();
|
|
await db.insert(companies).values({ id: other, name: "Other", issuePrefix: `F${other.slice(0, 7)}` });
|
|
await db.update(agents).set({ companyId: other }).where(eq(agents.id, agentId));
|
|
expect((await qualify()).status).toBe(404);
|
|
expect(probe).not.toHaveBeenCalled();
|
|
});
|
|
it("treats the submitted environment as a stale-state check, not a target override", async () => {
|
|
const res = await request(app).post(`/api/agents/${agentId}/qualify-runner`).send({ expectedEnvironmentId: randomUUID() });
|
|
expect(res.status).toBe(409);
|
|
expect(probe).not.toHaveBeenCalled();
|
|
});
|
|
it("rejects a managed computer bound to a different company", async () => {
|
|
const other = randomUUID();
|
|
await db.insert(companies).values({ id: other, name: "Other", issuePrefix: `F${other.slice(0, 7)}` });
|
|
await db.insert(builtInManagedResources).values({ companyId: other, bundleKey: "test", resourceKind: "environment", resourceKey: "computer", resourceId: environmentId, stockVersion: "test", stockHash: "test" });
|
|
expect((await qualify()).status).toBe(403);
|
|
expect(probe).not.toHaveBeenCalled();
|
|
});
|
|
it.each(["instance", "managed", "kubernetes"])("resolves the %s computer with the same policy as runner saving", async mode => {
|
|
await db.update(agents).set({ defaultEnvironmentId: null }).where(eq(agents.id, agentId));
|
|
await db.update(environments).set({ metadata: { managedByPaperclip: true, managedKubernetesSandbox: mode === "kubernetes" },
|
|
...(mode === "kubernetes" ? { config: { provider: "kubernetes" } } : {}) }).where(eq(environments.id, environmentId));
|
|
await db.insert(instanceSettings).values({ defaultEnvironmentId: mode === "instance" ? environmentId : null,
|
|
experimental: { enableManagedSandboxOnly: mode === "managed" }, general: { executionMode: mode === "kubernetes" ? "kubernetes" : "local" } });
|
|
expect((await qualify()).status).toBe(200);
|
|
expect(probe).toHaveBeenCalledWith(db, expect.objectContaining({ id: environmentId }), expect.anything());
|
|
});
|
|
it("does not accept arbitrary provider config or a target override", async () => {
|
|
const res = await request(app).post(`/api/agents/${agentId}/qualify-runner`).send({ expectedEnvironmentId: environmentId, config: { image: "untrusted" } });
|
|
expect(res.status).toBe(400);
|
|
expect(probe).not.toHaveBeenCalled();
|
|
});
|
|
it.each(["archived", "local"])("rejects an ineligible %s environment", async state => {
|
|
await db.update(environments).set(state === "archived" ? { status: "archived" } : { driver: "local", config: {} }).where(eq(environments.id, environmentId));
|
|
expect((await qualify()).status).toBe(422);
|
|
expect(probe).not.toHaveBeenCalled();
|
|
});
|
|
it("returns a failed qualification without exposing provider diagnostics or changing the agent", async () => {
|
|
probe.mockResolvedValue({ ok: false, driver: "sandbox", summary: "provider-secret", details: { error: "provider-secret" } });
|
|
const res = await qualify();
|
|
expect(res.status).toBe(200);
|
|
expect(res.body.ok).toBe(false);
|
|
expect(JSON.stringify(res.body)).not.toContain("provider-secret");
|
|
expect((await db.select().from(agents).where(eq(agents.id, agentId)))[0]?.adapterType).toBe("claude_local");
|
|
});
|
|
});
|