mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 16:27:18 +02:00
fix(auth): let Cloud stack admins manage experimental settings (#16907)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - People use experimental settings to opt into product features. > - Cloud stack Admins can manage their company, but cannot save an experiment. > - Requiring an Owner role adds unrelated privileges to this workflow. > - This pull request gives trusted Cloud stack Admins permission to update experiments. > - Other instance administration and platform settings keep their existing checks. ## Linked Issues or Issue Description **What happened?** A person with the Cloud stack Admin role receives “Instance admin access required” when they enable an experimental setting, such as Fast responses. **Expected behavior** Cloud stack Admins can save editable experimental settings. This permission does not grant general instance-admin authority. Members, Support users, and agents cannot change experiments. Cloud-managed and operator-hidden settings remain protected. **Steps to reproduce** 1. Sign into a Cloud staging workspace as a stack Admin. 2. Open Settings → Experimental. 3. Enable Experimental fast responses. 4. Observe the permission error instead of a saved setting. **Additional context** Related: #10343 introduced computed Owner elevation. #15748 added Fast responses and the company Connections settings page. The roadmap and existing PRs were checked. This is a small authorization fix for the existing settings path. ## What Changed - Derive experiment-management authority from the current trusted Cloud stack Admin role on each request. - Apply that authority only to the experimental settings PATCH route. Keep general settings, environment defaults, and task drain restricted to instance admins. - Preserve existing hidden-setting checks, managed feature restrictions, and activity records. - Add role and role-downgrade regressions, allowed and denied route cases, staging coverage, and updated OpenAPI and access documentation. ## Verification - Before the fix, the Admin fast-response request returned 403 and the new role assertions failed. The first sandboxed HTTP run could not bind local sockets; the regression run outside that sandbox reached the expected authorization failures. - Focused Cloud actor, settings route, managed-overlay, and health tests: 149 passed. - `node scripts/check-maps.mjs` and `git diff --check` pass. - `pnpm -r typecheck` and `pnpm build` pass. - Greptile reviewed commit `7c33b90066121f03183579d2fae2650755e025e4` at 5/5. Its one style finding is fixed and resolved. - The full local `pnpm test:run` did not pass. It reported two early collection failures before workspace build outputs were ready and a 15-second timeout in the unchanged AgentMail recovery test. The run was stopped after those failures while the full CI suite continued. No full local-suite pass is claimed. - The first current-head CI attempt failed the unchanged Runner test “preserves a real listening process and its marker when a health RPC times out” with `runner_remote_process_identity_unavailable` / `process_identity_changed`. The test passes in a local isolated run. This recovery does not establish the CI failure's cause. [Failed job](https://github.com/paperclipai/paperclip/actions/runs/39332989655/job/118336142261). The failed shard passed on its single rerun. The failure cause remains unconfirmed. All CI checks on commit `7c33b90066121f03183579d2fae2650755e025e4` are now green. [Final CI run](https://github.com/paperclipai/paperclip/actions/runs/39332989655). - No deployed staging behavior is claimed. Manual acceptance after deployment: as a stack Admin, enable Fast responses, reload, and confirm Connections appears. Confirm Members and Support users still cannot save experiments. ## Risks - This intentionally extends experimental settings write access to Cloud stack Admins. It does not persist an instance-admin role or expand other instance controls. - Production retains its existing managed feature policy. Staging retains its editable experiment allowlist and protected execution and owner-elevation settings. - No database migration or UI change is required. ## Model Used OpenAI Codex, based on GPT-6, with reasoning, repository tools, code execution, and browser inspection. The runtime does not expose an exact model snapshot or context-window size. ## 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>
This commit is contained in:
1 parent
8723580071
commit
d9bcf4dfe8
9 files changed
+127
-12
No files matched your search
@@ -520,6 +520,60 @@ describe("instance settings routes", () => {
|
||||
expect(mockInstanceSettingsService.updateExperimental).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe("Cloud stack admin experimental settings access", () => {
|
||||
afterEach(() => vi.unstubAllEnvs());
|
||||
|
||||
const stackAdmin = {
|
||||
type: "board",
|
||||
userId: "stack-admin-1",
|
||||
source: "cloud_tenant",
|
||||
companyIds: ["company-1"],
|
||||
isInstanceAdmin: false,
|
||||
canManageExperimentalSettings: true,
|
||||
};
|
||||
|
||||
it("allows a Cloud stack admin to enable fast responses and audits the change", async () => {
|
||||
const app = createApp(stackAdmin);
|
||||
await request(app).patch("/api/instance/settings/experimental")
|
||||
.send({ enableFastResponses: true }).expect(200);
|
||||
expect(mockInstanceSettingsService.updateExperimental).toHaveBeenCalledWith({ enableFastResponses: true });
|
||||
expect(mockLogActivity).toHaveBeenCalledWith(mockDb, expect.objectContaining({
|
||||
actorId: "stack-admin-1",
|
||||
action: "instance.settings.experimental_updated",
|
||||
details: expect.objectContaining({ changedKeys: ["enableFastResponses"] }),
|
||||
}));
|
||||
});
|
||||
|
||||
it.each([
|
||||
["patch", "/api/instance/settings", { defaultEnvironmentId: null }],
|
||||
["patch", "/api/instance/settings/general", { censorUsernameInLogs: true }],
|
||||
["post", "/api/instance/task-drain", {}],
|
||||
] as const)("keeps %s %s restricted to instance admins", async (method, path, body) => {
|
||||
const app = createApp(stackAdmin);
|
||||
await request(app)[method](path).send(body).expect(403);
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ ...stackAdmin, source: "session" },
|
||||
{ ...stackAdmin, source: "mcp_oauth" },
|
||||
{ ...stackAdmin, type: "agent" },
|
||||
{ ...stackAdmin, canManageExperimentalSettings: false },
|
||||
])("rejects actors without trusted Cloud experimental authority (%j)", async (actor) => {
|
||||
const app = createApp(actor);
|
||||
await request(app).patch("/api/instance/settings/experimental")
|
||||
.send({ enableFastResponses: true }).expect(403);
|
||||
expect(mockInstanceSettingsService.updateExperimental).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps operator-hidden experimental settings locked for stack admins", async () => {
|
||||
vi.stubEnv("PAPERCLIP_HIDDEN_SETTINGS", "instance.experimental.enableFastResponses");
|
||||
const app = createApp(stackAdmin);
|
||||
await request(app).patch("/api/instance/settings/experimental")
|
||||
.send({ enableFastResponses: true }).expect(403);
|
||||
expect(mockInstanceSettingsService.updateExperimental).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
it("allows local board users to read and update general settings", async () => {
|
||||
const app = await createApp(localAdminActor());
|
||||
|
||||
@@ -878,7 +932,10 @@ describe("instance settings routes", () => {
|
||||
vi.stubEnv("PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN", "tenant-token");
|
||||
vi.stubEnv("PAPERCLIP_PRIMARY_HOST", "team.staging.paperclip.app");
|
||||
vi.stubEnv("PAPERCLIP_HIDDEN_SETTINGS", "instance.experimental,instance.experimental.*,instance.general.censorUsernameInLogs");
|
||||
const app = createApp({ type: "board", userId: "admin-1", source: "session", isInstanceAdmin: true });
|
||||
const app = createApp({
|
||||
type: "board", userId: "admin-1", source: "cloud_tenant",
|
||||
isInstanceAdmin: false, canManageExperimentalSettings: true,
|
||||
});
|
||||
await request(app).patch("/api/instance/settings/experimental").send({ enableChatConnectors: true }).expect(200);
|
||||
expect(mockInstanceSettingsService.updateExperimental).toHaveBeenCalledWith({ enableChatConnectors: true });
|
||||
await request(app).patch("/api/instance/settings/general").send({ censorUsernameInLogs: true }).expect(403);
|
||||
@@ -895,7 +952,7 @@ describe("instance settings routes", () => {
|
||||
expect(mockInstanceSettingsService.updateExperimental).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("still requires instance admin access on staging", async () => {
|
||||
it("still denies ordinary board members on staging", async () => {
|
||||
vi.stubEnv("PAPERCLIP_CLOUD_TENANT_SERVER_TOKEN", "tenant-token");
|
||||
vi.stubEnv("PAPERCLIP_PRIMARY_HOST", "team.staging.paperclip.app");
|
||||
const app = createApp({ type: "board", userId: "member-1", source: "session", isInstanceAdmin: false });
|
||||
|
||||
@@ -788,6 +788,7 @@ async function resolveCloudTenantActorOnce(
|
||||
// company-scoped. Turning the flag off de-elevates on the next request —
|
||||
// there is no role row to clean up.
|
||||
isInstanceAdmin: await resolveOwnerInstanceAdmin(db, stackRole),
|
||||
canManageExperimentalSettings: stackRole === "admin",
|
||||
source: "cloud_tenant",
|
||||
};
|
||||
}
|
||||
|
||||
@@ -270,6 +270,34 @@ describe("resolveCloudTenantActor (shared-pool hardening)", () => {
|
||||
expect(actor?.memberships?.[0]?.membershipRole).toBe("member");
|
||||
});
|
||||
|
||||
it.each(["admin", "member", "support"])(
|
||||
"grants experimental settings access only to the admin stack role (%s)",
|
||||
async (stackRole) => {
|
||||
const { db, insertedTables } = createFakeDb();
|
||||
const actor = await resolveCloudTenantActor(
|
||||
db,
|
||||
fakeReq({ ...VALID_HEADERS, "x-paperclip-cloud-stack-role": stackRole }),
|
||||
);
|
||||
expect(actor?.canManageExperimentalSettings).toBe(stackRole === "admin");
|
||||
expect(actor?.isInstanceAdmin).toBe(false);
|
||||
expect(insertedTables).not.toContain(instanceUserRoles);
|
||||
},
|
||||
);
|
||||
|
||||
it("removes experimental settings access on the next request after a stack admin is demoted", async () => {
|
||||
const { db } = createFakeDb();
|
||||
const admin = await resolveCloudTenantActor(
|
||||
db,
|
||||
fakeReq({ ...VALID_HEADERS, "x-paperclip-cloud-stack-role": "admin" }),
|
||||
);
|
||||
const member = await resolveCloudTenantActor(
|
||||
db,
|
||||
fakeReq({ ...VALID_HEADERS, "x-paperclip-cloud-stack-role": "member" }),
|
||||
);
|
||||
expect(admin?.canManageExperimentalSettings).toBe(true);
|
||||
expect(member?.canManageExperimentalSettings).toBe(false);
|
||||
});
|
||||
|
||||
describe("company membership union", () => {
|
||||
// The primary company id is derived from the stack id; resolve it once
|
||||
// through the same code path instead of duplicating the hash here.
|
||||
|
||||
@@ -96,6 +96,17 @@ function assertCanManageInstanceSettings(req: Request) {
|
||||
throw forbidden("Instance admin access required");
|
||||
}
|
||||
|
||||
function assertCanManageExperimentalSettings(req: Request) {
|
||||
if (
|
||||
req.actor.type === "board"
|
||||
&& req.actor.source === "cloud_tenant"
|
||||
&& req.actor.canManageExperimentalSettings
|
||||
) {
|
||||
return;
|
||||
}
|
||||
assertCanManageInstanceSettings(req);
|
||||
}
|
||||
|
||||
// A task-drain start or stop reads the live drain state, writes an audit
|
||||
// transaction, and only then mutates the process-local drain state. The
|
||||
// audit write is an async gap: two overlapping requests can commit their
|
||||
@@ -252,9 +263,6 @@ export function instanceSettingsRoutes(db: Db, pluginWorkers?: PluginWorkerManag
|
||||
);
|
||||
|
||||
router.get("/instance/settings/experimental", async (req, res) => {
|
||||
// Experimental settings are readable by any authenticated org member
|
||||
// or instance admin. Updating them remains instance-admin only because
|
||||
// this payload includes instance-wide operational controls.
|
||||
assertBoardOrgAccess(req);
|
||||
res.json(await svc.getExperimental());
|
||||
});
|
||||
@@ -263,7 +271,7 @@ export function instanceSettingsRoutes(db: Db, pluginWorkers?: PluginWorkerManag
|
||||
"/instance/settings/experimental",
|
||||
validate(patchInstanceExperimentalSettingsSchema),
|
||||
async (req, res) => {
|
||||
assertCanManageInstanceSettings(req);
|
||||
assertCanManageExperimentalSettings(req);
|
||||
// Hiding the whole Experimental page floors every toggle; otherwise
|
||||
// only individually hidden keys are floored.
|
||||
const hidden = getHiddenSettings();
|
||||
|
||||
@@ -6894,8 +6894,9 @@ registry.registerPath({
|
||||
path: "/api/instance/settings/experimental",
|
||||
tags: ["instance"],
|
||||
summary: "Update experimental instance settings",
|
||||
description: "Requires instance-admin access or a trusted Cloud stack admin. Operator-hidden settings and Cloud-managed feature restrictions still apply. This does not grant access to other instance administration operations.",
|
||||
request: { body: jsonBody(patchInstanceExperimentalSettingsSchema) },
|
||||
responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized },
|
||||
responses: { 200: r.ok(), 400: r.badRequest, 401: r.unauthorized, 403: r.forbidden },
|
||||
});
|
||||
|
||||
registry.registerPath({
|
||||
|
||||
Vendored
+1
@@ -25,6 +25,7 @@ declare global {
|
||||
status?: string;
|
||||
}>;
|
||||
isInstanceAdmin?: boolean;
|
||||
canManageExperimentalSettings?: boolean;
|
||||
keyId?: string;
|
||||
keyScope?: AgentApiKeyScope;
|
||||
runId?: string;
|
||||
|
||||
Reference in new issue
Block a user