mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 13:24:55 +01:00
## Thinking Path > - Paperclip is the control plane for autonomous AI companies > - Its agents and adapters need to resolve secrets through the same governed runtime path that checks ownership and company boundaries > - This change fixes a gap where user-scoped secret resolution could lose the acting-user context before adapter runtime startup > - Without that context, a required user secret could fail closed with responsible_user_missing even though an authenticated user was in scope > - This PR threads the acting user into the user-scoped secret resolution path and keeps the owner boundary explicit > - The benefit is adapter runtime setup can resolve the right credential without broadening access ## Linked Issues or Issue Description Refs #8309 (related: agent secret_ref env drift and binding context) No exact public GitHub issue for this specific behavior. ### Bug report - Problem: two agent-management routes resolved user-scoped secrets without an acting-user binding, so a required `user_secret_ref` could not be resolved before runtime. - Expected behavior: the authenticated acting user should be threaded into user-scoped secret resolution so the owning user secret can be selected safely. - Actual behavior: adapter startup paths failed closed with `responsible_user_missing` even though a user was already in scope. - Steps to reproduce: configure an adapter test-environment or login flow that depends on a user-scoped secret, then invoke it with an authenticated user context that does not carry the acting-user binding into runtime secret resolution. - Impact: the adapter test-environment probe and login path cannot start, so the runtime never reaches the work it was supposed to do. ## What Changed - Added an actor secret-context helper so the server can derive responsible-user context without inventing config-path or binding allowlists. - Added an explicit user-secret mediation mode for runtime config resolution, with an owner-scoped path that resolves by definition plus owner boundary and fails closed when an allowlist is present. - Wired the adapter test-environment route to owner-scoped mediation with an audit-only consumer and kept claude-login on the declared path with its persisted agent identity. - Added and updated tests for the factory, owner-scoped resolver mode, and adapter route coverage. ## Verification - `tsc --noEmit` clean - Factory tests: `authz-secret-context` 5/5 - Service tests: `secrets-service-user-secret-owner-scoped` 5/5, including fail-closed allowlist coverage and company-secret non-regression - Route tests: `agents-adapter-config-user-secret` 5/5, including `responsible_user_missing` and `binding_missing` coverage - Regression suites: `agents` + `secrets` 194/194 ## Risks - A regression in the owner-scoped mediation path could accidentally loosen secret access if the audit consumer or allowlist guard changes. - The change depends on the server-derived responsible user; if auth context regresses, the system should fail closed with responsible_user_missing. - The new mediation mode adds a branch in runtime config resolution, so future changes need to keep declared-mode behavior intact. ## Model Used - OpenAI GPT-5 (Codex tool-use 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 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: Harold Kim <harold@paperclip.ing> Co-authored-by: Paperclip <noreply@paperclip.ing>
290 lines
10 KiB
TypeScript
290 lines
10 KiB
TypeScript
import type { Request, Response } from "express";
|
|
import type { SecretBindingTargetType } from "@paperclipai/shared";
|
|
import { forbidden, HttpError, unauthorized } from "../errors.js";
|
|
import { logger } from "../middleware/logger.js";
|
|
import { responsibleUserAuthzShadowMode } from "../services/authorization.js";
|
|
|
|
function throwOrShadowResponsibleUserCompanyAccessDeny(
|
|
req: Request,
|
|
companyId: string,
|
|
code: "RESPONSIBLE_USER_UNAUTHORIZED" | "RESPONSIBLE_USER_UNAVAILABLE",
|
|
message: string,
|
|
) {
|
|
logger.warn({
|
|
authzMode: responsibleUserAuthzShadowMode() ? "shadow" : "enforce",
|
|
code,
|
|
action: "company_access",
|
|
companyId,
|
|
actorAgentId: req.actor.agentId ?? null,
|
|
responsibleUserId: req.actor.onBehalfOfUserId ?? null,
|
|
method: req.method,
|
|
}, "responsible-user company access intersection denied");
|
|
if (responsibleUserAuthzShadowMode()) return;
|
|
throw new HttpError(403, message, { code });
|
|
}
|
|
|
|
export function assertAuthenticated(req: Request) {
|
|
if (req.actor.type === "none") {
|
|
throw unauthorized();
|
|
}
|
|
}
|
|
|
|
export function assertBoard(req: Request) {
|
|
if (req.actor.type !== "board") {
|
|
throw forbidden("Board access required");
|
|
}
|
|
}
|
|
|
|
export function hasBoardOrgAccess(req: Request) {
|
|
if (req.actor.type !== "board") {
|
|
return false;
|
|
}
|
|
if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) {
|
|
return true;
|
|
}
|
|
return Array.isArray(req.actor.companyIds) && req.actor.companyIds.length > 0;
|
|
}
|
|
|
|
export function assertBoardOrgAccess(req: Request) {
|
|
assertBoard(req);
|
|
if (hasBoardOrgAccess(req)) {
|
|
return;
|
|
}
|
|
throw forbidden("Company membership or instance admin access required");
|
|
}
|
|
|
|
export function assertBoardOrAgent(req: Request) {
|
|
if (req.actor.type === "agent") {
|
|
return;
|
|
}
|
|
if (req.actor.type === "board") {
|
|
assertBoardOrgAccess(req);
|
|
return;
|
|
}
|
|
throw forbidden("Board or agent access required");
|
|
}
|
|
|
|
export function assertInstanceAdmin(req: Request) {
|
|
assertBoard(req);
|
|
if (req.actor.source === "local_implicit" || req.actor.isInstanceAdmin) {
|
|
return;
|
|
}
|
|
throw forbidden("Instance admin access required");
|
|
}
|
|
|
|
export function assertCompanyAccess(req: Request, companyId: string) {
|
|
assertAuthenticated(req);
|
|
if (req.actor.type === "agent" && req.actor.companyId !== companyId) {
|
|
throw forbidden("Agent key cannot access another company");
|
|
}
|
|
if (req.actor.type === "agent" && req.actor.onBehalfOfUserId?.trim()) {
|
|
const membership = req.actor.onBehalfOfMemberships?.find(
|
|
(item) => item.companyId === companyId && item.status === "active",
|
|
);
|
|
if (!membership) {
|
|
throwOrShadowResponsibleUserCompanyAccessDeny(
|
|
req,
|
|
companyId,
|
|
"RESPONSIBLE_USER_UNAVAILABLE",
|
|
"Responsible user is unavailable for this company",
|
|
);
|
|
return;
|
|
}
|
|
const method = typeof req.method === "string" ? req.method.toUpperCase() : "GET";
|
|
const isSafeMethod = ["GET", "HEAD", "OPTIONS"].includes(method);
|
|
if (!isSafeMethod && membership.membershipRole === "viewer") {
|
|
throwOrShadowResponsibleUserCompanyAccessDeny(
|
|
req,
|
|
companyId,
|
|
"RESPONSIBLE_USER_UNAUTHORIZED",
|
|
"Responsible user is not authorized for write access",
|
|
);
|
|
}
|
|
}
|
|
if (req.actor.type === "board" && req.actor.source !== "local_implicit") {
|
|
const allowedCompanies = req.actor.companyIds ?? [];
|
|
if (!allowedCompanies.includes(companyId)) {
|
|
throw forbidden("User does not have access to this company");
|
|
}
|
|
const method = typeof req.method === "string" ? req.method.toUpperCase() : "GET";
|
|
const isSafeMethod = ["GET", "HEAD", "OPTIONS"].includes(method);
|
|
if (!isSafeMethod && !req.actor.isInstanceAdmin && Array.isArray(req.actor.memberships)) {
|
|
const membership = req.actor.memberships.find((item) => item.companyId === companyId);
|
|
if (!membership || membership.status !== "active") {
|
|
throw forbidden("User does not have active company access");
|
|
}
|
|
if (membership.membershipRole === "viewer") {
|
|
throw forbidden("Viewer access is read-only");
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Non-throwing access check for routes that look up a resource by id
|
|
* before responding. Prefer this over `assertCompanyAccess` whenever the
|
|
* route can reach the access check only after a successful `getById`
|
|
* (i.e. after confirming the resource exists).
|
|
*
|
|
* Using `assertCompanyAccess` in that position leaks resource existence
|
|
* across tenants: a 404 means "no such resource" while a 403 means "exists
|
|
* in another tenant". Any authenticated user can enumerate IDs and
|
|
* distinguish the two responses.
|
|
*
|
|
* Most routes should use `getAccessibleResource` below, which wraps the
|
|
* whole pattern. When composing manually (bespoke not-found responses),
|
|
* the shape is:
|
|
*
|
|
* const issue = await svc.getById(id);
|
|
* if (!issue || !hasCompanyAccess(req, issue.companyId)) {
|
|
* res.status(404).json({ error: "Issue not found" });
|
|
* return;
|
|
* }
|
|
*
|
|
* so both "does not exist" and "exists but cross-tenant" return the same
|
|
* 404, removing the oracle.
|
|
*
|
|
* Note: this intentionally does not replicate the write-path membership
|
|
* checks in `assertCompanyAccess` (active membership, viewer read-only).
|
|
* Routes that need those checks for authorized tenants should still call
|
|
* `assertCompanyAccess` after the 404 gate — the oracle concern is only
|
|
* about the existence check.
|
|
*
|
|
* The company-scope semantics must stay in lockstep with
|
|
* `assertCompanyAccess`: in particular, signed-in instance admins do NOT
|
|
* get blanket access to companies they are not a member of.
|
|
*/
|
|
export function hasCompanyAccess(req: Request, companyId: string): boolean {
|
|
if (req.actor.type === "none") return false;
|
|
if (req.actor.type === "agent") return req.actor.companyId === companyId;
|
|
if (req.actor.source === "local_implicit") return true;
|
|
return (req.actor.companyIds ?? []).includes(companyId);
|
|
}
|
|
|
|
/**
|
|
* Preferred way to fetch a company-scoped resource by id inside a route
|
|
* handler. Wraps the two-step pattern described on `hasCompanyAccess` so
|
|
* new routes cannot accidentally reintroduce the existence oracle:
|
|
*
|
|
* - missing resource → 404 `{ error: notFoundMessage }`, returns null
|
|
* - exists but cross-tenant → identical 404, returns null
|
|
* - accessible → runs `assertCompanyAccess` (write-path
|
|
* membership checks on non-safe methods) and returns the resource
|
|
*
|
|
* Usage:
|
|
*
|
|
* const goal = await getAccessibleResource(req, res, svc.getById(id), "Goal not found");
|
|
* if (!goal) return;
|
|
*
|
|
* Routes with bespoke not-found behavior (legacy `200 []` contracts,
|
|
* audit-logged denials) should still compose `hasCompanyAccess` directly.
|
|
*/
|
|
export async function getAccessibleResource<T extends { companyId: string }>(
|
|
req: Request,
|
|
res: Response,
|
|
resource: T | null | undefined | Promise<T | null | undefined>,
|
|
notFoundMessage: string,
|
|
): Promise<T | null> {
|
|
const resolved = await resource;
|
|
if (!resolved || !hasCompanyAccess(req, resolved.companyId)) {
|
|
res.status(404).json({ error: notFoundMessage });
|
|
return null;
|
|
}
|
|
assertCompanyAccess(req, resolved.companyId);
|
|
return resolved;
|
|
}
|
|
|
|
export function getActorInfo(req: Request): (
|
|
{
|
|
actorType: "agent";
|
|
actorId: string;
|
|
agentId: string | null;
|
|
runId: string | null;
|
|
agentApiKeyId: string | null;
|
|
actorSource: "agent_key" | "agent_jwt";
|
|
}
|
|
| {
|
|
actorType: "user";
|
|
actorId: string;
|
|
sessionId: string | null;
|
|
agentId: null;
|
|
runId: string | null;
|
|
agentApiKeyId: null;
|
|
actorSource: "local_implicit" | "session" | "board_key" | "cloud_tenant";
|
|
}
|
|
) {
|
|
assertAuthenticated(req);
|
|
if (req.actor.type === "agent") {
|
|
const actorSource = req.actor.source === "agent_jwt" ? "agent_jwt" : "agent_key";
|
|
return {
|
|
actorType: "agent" as const,
|
|
actorId: req.actor.agentId ?? "unknown-agent",
|
|
agentId: req.actor.agentId ?? null,
|
|
runId: req.actor.runId ?? null,
|
|
agentApiKeyId: req.actor.keyId ?? null,
|
|
actorSource,
|
|
};
|
|
}
|
|
|
|
const actorSource =
|
|
req.actor.source === "local_implicit" ||
|
|
req.actor.source === "board_key" ||
|
|
req.actor.source === "cloud_tenant"
|
|
? req.actor.source
|
|
: "session";
|
|
|
|
return {
|
|
actorType: "user" as const,
|
|
actorId: req.actor.userId ?? "board",
|
|
sessionId: req.actor.sessionId ?? null,
|
|
agentId: null,
|
|
runId: req.actor.runId ?? null,
|
|
agentApiKeyId: null,
|
|
actorSource,
|
|
};
|
|
}
|
|
|
|
/**
|
|
* The actor-scoped fields of a secret-binding context, keyed to a caller-supplied
|
|
* consumer identity. Structurally matches `SecretConsumerContext` in
|
|
* `services/secrets.ts` (whose types are not exported), so the return value slots
|
|
* into `resolveAdapterConfigForRuntime`'s 3rd argument
|
|
* (`Omit<SecretBindingContext, "configPath">`) unchanged.
|
|
*/
|
|
export type ActorSecretContext = {
|
|
consumerType: SecretBindingTargetType;
|
|
consumerId: string;
|
|
actorType: "agent" | "user";
|
|
actorId: string | null;
|
|
actorSource: "local_implicit" | "session" | "board_key" | "agent_key" | "agent_jwt" | "cloud_tenant";
|
|
responsibleUserId: string | null;
|
|
};
|
|
|
|
/**
|
|
* Build the actor-scoped portion of a secret-binding context from `req.actor`,
|
|
* taking the consumer identity as parameters. The responsible user is derived
|
|
* server-side (`req.actor.userId ?? req.actor.onBehalfOfUserId ?? null`) and is
|
|
* never request-body-controllable; a `null` result surfaces downstream as the
|
|
* intended `responsible_user_missing` loud failure for a required user secret.
|
|
*
|
|
* `consumerType` is a parameter (not hardcoded `"agent"`) so callers can record an
|
|
* honest consumer — `agent` for a persisted agent, `environment`/`system` for a
|
|
* prospective config with no persisted consumer.
|
|
*
|
|
* Never sets `configPath` (the resolver injects it) or `allowedBindingIds`.
|
|
*/
|
|
export function buildActorSecretContext(
|
|
req: Request,
|
|
params: { consumerType: SecretBindingTargetType; consumerId: string },
|
|
): ActorSecretContext {
|
|
const info = getActorInfo(req);
|
|
return {
|
|
consumerType: params.consumerType,
|
|
consumerId: params.consumerId,
|
|
actorType: info.actorType,
|
|
actorId: info.actorId,
|
|
actorSource: info.actorSource,
|
|
responsibleUserId: req.actor.userId ?? req.actor.onBehalfOfUserId ?? null,
|
|
};
|
|
}
|