RomanNum3ral
/
paperclip
mirror of https://github.com/paperclipai/paperclip.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

fix(security): close board key review gaps 

Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
cryppadotta 2026-09-12 15:03:42 +00:00
parent c3bd574547
commit a2bd9b7a36
6 changed files with 68 additions and 77 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

71
packages/db/src/migrations/0278_natural_nehzno.sql
View File

@ -1,73 +1,2 @@
ALTER TABLE "principal_permission_grants" ADD COLUMN "grant_origin" text DEFAULT 'explicit' NOT NULL;--> statement-breakpoint
UPDATE "principal_permission_grants" grants
SET "grant_origin" = 'role_default'
FROM "company_memberships" memberships
WHERE grants."company_id" = memberships."company_id"
AND grants."principal_type" = 'user'
AND grants."principal_id" = memberships."principal_id"
AND memberships."principal_type" = 'user'
AND memberships."status" = 'active'
AND grants."scope" IS NULL
AND grants."granted_by_user_id" IS NULL
AND NOT EXISTS (
SELECT 1
FROM "activity_log" explicit_activity
WHERE explicit_activity."company_id" = grants."company_id"
AND (
(
explicit_activity."action" = 'authorization.grants_updated_by_plugin'
AND explicit_activity."entity_type" = 'principal_permission_grants'
AND explicit_activity."entity_id" = grants."principal_type" || ':' || grants."principal_id"
)
OR (
explicit_activity."action" = 'company_member.permissions_updated'
AND explicit_activity."entity_type" = 'company_membership'
AND explicit_activity."entity_id" = memberships."id"::text
)
)
)
AND NOT EXISTS (
SELECT 1
FROM "join_requests" approved_human_join
WHERE approved_human_join."company_id" = grants."company_id"
AND approved_human_join."request_type" = 'human'
AND approved_human_join."requesting_user_id" = grants."principal_id"
AND approved_human_join."status" = 'approved'
)
AND (
(memberships."membership_role" = 'owner' AND grants."permission_key" IN (
'agents:create',
'agents:configure',
'skills:create',
'environments:manage',
'users:invite',
'users:manage_permissions',
'tasks:assign',
'tasks:manage_active_checkouts',
'joins:approve',
'pipelines:write',
'audit:view_agent_actions',
'tools:manage_connections',
'tools:manage_runtime',
'tools:use',
'tools:admin'
))
OR (memberships."membership_role" = 'admin' AND grants."permission_key" IN (
'agents:create',
'agents:configure',
'skills:create',
'environments:manage',
'users:invite',
'tasks:assign',
'tasks:manage_active_checkouts',
'joins:approve',
'pipelines:write',
'audit:view_agent_actions',
'tools:manage_connections',
'tools:manage_runtime',
'tools:use',
'tools:admin'
))
OR (memberships."membership_role" IN ('member', 'operator') AND grants."permission_key" = 'tasks:assign')
);--> statement-breakpoint
ALTER TABLE "principal_permission_grants" ADD CONSTRAINT "principal_permission_grants_origin_check" CHECK ("principal_permission_grants"."grant_origin" in ('explicit', 'role_default'));

8
packages/db/src/principal-permission-grant-origin-migration.test.ts
View File

@ -27,7 +27,7 @@ async function migrationStatements() {
}
describeEmbeddedPostgres("principal permission grant origin migration", () => {
it("marks historical role defaults without consuming explicit grants", async () => {
it("preserves ambiguous historical grants as explicit", async () => {
const database = await startEmbeddedPostgresTestDatabase("paperclip-grant-origin-");
cleanups.push(database.cleanup);
const sql = postgres(database.connectionString, { max: 1, onnotice: () => {} });
@ -102,9 +102,9 @@ describeEmbeddedPostgres("principal permission grant origin migration", () => {
ORDER BY id
`);
expect(rows).toEqual([
{ id: "00000000-0000-4000-8000-000000000010", grant_origin: "role_default" },
{ id: "00000000-0000-4000-8000-000000000011", grant_origin: "role_default" },
{ id: "00000000-0000-4000-8000-000000000012", grant_origin: "role_default" },
{ id: "00000000-0000-4000-8000-000000000010", grant_origin: "explicit" },
{ id: "00000000-0000-4000-8000-000000000011", grant_origin: "explicit" },
{ id: "00000000-0000-4000-8000-000000000012", grant_origin: "explicit" },
{ id: "00000000-0000-4000-8000-000000000013", grant_origin: "explicit" },
{ id: "00000000-0000-4000-8000-000000000014", grant_origin: "explicit" },
{ id: "00000000-0000-4000-8000-000000000015", grant_origin: "explicit" },

30
server/src/__tests__/access-routes-permissions-upgrade.test.ts
View File

@ -231,6 +231,36 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () =
)).resolves.toBe(false);
});
it("rejects grant-backed board-key authority after membership suspension", async () => {
const { company, owner } = await createCompanyWithOwner(db);
await db.insert(principalPermissionGrants).values({
companyId: company.id,
principalType: "user",
principalId: owner.principalId,
permissionKey: "tools:admin",
grantOrigin: "explicit",
});
await expect(ownerHasRequiredGrant(
db,
owner.principalId,
[company.id],
"tools:manage",
)).resolves.toBe(true);
await db
.update(companyMemberships)
.set({ status: "suspended" })
.where(eq(companyMemberships.id, owner.id));
await expect(ownerHasRequiredGrant(
db,
owner.principalId,
[company.id],
"tools:manage",
)).resolves.toBe(false);
});
it("sweeps personal connection access when the member route suspends a user", async () => {
const { company, owner } = await createCompanyWithOwner(db);
const member = await db.insert(companyMemberships).values({

24
server/src/__tests__/board-key-auth-middleware.test.ts
View File

@ -122,9 +122,10 @@ function createDbState() {
function createApp(
db: any,
resolveSession = vi.fn(async () => null),
options: { sourceFromHeader?: boolean } = {},
options: { sourceFromHeader?: boolean; trustProxy?: boolean } = {},
) {
const app = express();
if (options.trustProxy) app.set("trust proxy", true);
if (options.sourceFromHeader) {
app.use((req, _res, next) => {
Object.defineProperty(req.socket, "remoteAddress", {
@ -222,6 +223,27 @@ describe("board-key authentication middleware", () => {
expect(db.select.mock.calls.length).toBe(lookupsBeforeThrottle);
});
it("isolates failure buckets for clients behind a trusted shared proxy", async () => {
const { db, state } = createDbState();
state.keyExists = false;
const { app } = createApp(db, undefined, { trustProxy: true });
const sourceLimit = BOARD_KEY_AUTH_FAILURE_RATE_LIMIT_DEFAULTS.sourceLimit;
for (let index = 0; index < sourceLimit; index += 1) {
const response = await request(app)
.get("/actor")
.set("x-forwarded-for", "198.51.100.10")
.set("Authorization", `Bearer pcp_board_proxy_a_${index}`);
expect(response.status).toBe(401);
}
const unrelatedClient = await request(app)
.get("/actor")
.set("x-forwarded-for", "198.51.100.11")
.set("Authorization", "Bearer pcp_board_proxy_b");
expect(unrelatedClient.status).toBe(401);
});
it("strictly bounds credential and source failure storage during identity floods", () => {
const limiter = createBoardKeyAuthFailureRateLimiter({
windowMs: 60_000,

6
server/src/middleware/auth.ts
View File

@ -68,7 +68,11 @@ function hashToken(token: string) {
const boardKeyAuthFailureRateLimiter = createBoardKeyAuthFailureRateLimiter();
function boardKeyAuthFailureSource(req: Request) {
return req.socket.remoteAddress || req.ip || "unknown";
// Express derives req.ip from the socket unless the immediate peer matches
// the operator's TRUST_PROXY policy. In the latter case it is the validated
// forwarded client address, so clients behind one proxy do not share a
// failure bucket merely because they share the proxy socket.
return req.ip || req.socket.remoteAddress || "unknown";
}
export function resetBoardKeyAuthFailureRateLimitForTests() {

6
server/src/security/board-key-owner-authority.ts
View File

@ -137,6 +137,12 @@ export async function ownerHasRequiredGrant(
permissionKey: principalPermissionGrants.permissionKey,
})
.from(principalPermissionGrants)
.innerJoin(companyMemberships, and(
eq(companyMemberships.companyId, principalPermissionGrants.companyId),
eq(companyMemberships.principalType, "user"),
eq(companyMemberships.principalId, ownerUserId),
eq(companyMemberships.status, "active"),
))
.where(and(
inArray(principalPermissionGrants.companyId, [...companyIds]),
eq(principalPermissionGrants.principalType, "user"),