mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-07 07:55:47 +01:00
5f8a3c8eec4e65d8aac2ab868c1f8efda35a13fe
11
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 DottaandPaperclip
|
9dd6526b47 |
fix(security): harden privileged server boundaries (#12776)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The server controls secrets, host files, outbound requests, and workspace commands > - A red-team review found cases where restricted callers could cross these trust boundaries > - These cases could expose credentials or let untrusted input reach privileged resources > - This pull request applies least-privilege checks at each affected server boundary > - The benefit is safer agent execution without changing the private-instance bootstrap contract ## Linked Issues or Issue Description **What happened?** Several server paths used authorization, redaction, or content-delivery rules that were too broad. Restricted agent keys could obtain company-level operational data. Some adapter and instruction paths could reach server-owned network or file resources without the required owner approval. **Expected behavior** Paperclip must redact credential values, enforce restricted-key scopes, guard outbound network access, prevent same-origin script execution, and reserve host-level file and command controls for authorized operators. **Steps to reproduce** 1. Configure an authenticated development instance at the parent commit. 2. Exercise the affected APIs with a restricted agent key or a non-instance-admin company user. 3. Observe that the parent commit returns privileged data or accepts a privileged operation. 4. Repeat on this branch and observe a redacted response, a safe download, or an HTTP 403 response. **Paperclip version or commit** The findings reproduce from commit `39898ab22` and are fixed by this pull request. **Deployment mode** Authenticated self-hosted server and local development modes. **Installation method** Built from source with pnpm. ## What Changed - Redact generic secret `value` and `token` fields recursively in structured logs. - Classify exact and separator-suffixed `KEY` environment names as secrets in company exports. - Limit restricted self-identity responses and protect company run, log, and secret catalog APIs. - Route HTTP adapter requests through DNS-pinned SSRF protection with exact private-origin allowlisting. - Download HTML, SVG, and other script-capable assets with `nosniff` and a sandbox CSP. - Require instance-admin access for external instruction roots and exports that read them. - Block agent-authenticated host command persistence across supported workspace runtime shapes. - Apply the central runtime-management decision before workspace command controls. - Keep the documented first-user instance-admin claim contract unchanged. - Add regression tests and server-owner configuration documentation. ## Verification - `pnpm -r typecheck` passes. - The Node 24 remediation suite passes with 365 tests. It skips 25 environment-gated tests. - `pnpm build` passes under Node 24. - `git diff --check` passes. - The full local runner reaches known macOS-only general-server harness failures before the serialized route lane. The Linux PR matrix is the authoritative full-suite gate. ## Risks - Restricted agent keys now receive HTTP 403 responses from company-wide run, log, and secret catalog endpoints. - Script-capable assets now download instead of rendering inline. - External instruction roots now require instance-admin access. - Private HTTP adapter endpoints now require an exact origin in `PAPERCLIP_HTTP_ADAPTER_PRIVATE_ENDPOINT_ALLOWLIST`. - Public HTTP adapter endpoints remain enabled. Redirects and metadata or link-local targets remain blocked. - No database migration is required. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5. The exact serving snapshot and context-window size are not exposed. The model used tool-enabled reasoning, repository access, code execution, and test execution. ## 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> |
||
|
DottaandPaperclip
|
6b8e42168e |
Add governed secret alias confirmation cards (#11486)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents need scoped secret bindings to use external services safely. > - Agents could not request an existing secret under a new config name without an internal secret identifier. > - Existing binding proposals were only visible in Settings and did not create an issue-thread approval path. > - A confirmation card could record acceptance without proving that the binding was created. > - This pull request extends the existing secret proposal system with safe source references and governed issue-thread confirmation cards. > - The benefit is a one-click flow that creates the binding or shows a clear failure without exposing secret material. ## Linked Issues or Issue Description Related prerequisite: #11482. **Subsystem affected** Cross-cutting: server REST APIs, shared interaction contracts, database proposal schema, and issue-thread UI. **Problem or motivation** An agent can need an existing bound secret under a second config name. The agent cannot safely discover the internal secret identifier. The existing proposal is also easy for the operator to miss because it only appears in Settings. A generic confirmation can record acceptance without executing the binding. **Proposed solution** Let an agent create a binding proposal from one of its existing config paths. Mint a server-owned, human-only confirmation card on the checked-out issue. Recheck the operator's target-agent permission under the proposal row lock. Execute the existing proposal transaction after card acceptance. Store an `executed` or `failed` result on the card. Render the complete lifecycle in the issue thread and attention resolver. **Alternatives considered** A new alias subsystem would duplicate proposal quotas, expiry, authorization, and binding synchronization. A text-only issue comment would not provide a governed action or an execution result. An agent-supplied card payload would permit metadata smuggling. This change uses the existing proposal transaction and a server-owned payload instead. **Roadmap alignment** This change extends the completed "Secrets Manager with per-agent access" roadmap item. It preserves scoped bindings and audited resolution. The required GitHub search found no other open duplicate issue or pull request. ## What Changed - Added safe source-config-path binding proposals and preserved user-secret ownership checks. - Added a proposal-to-interaction link and an idempotent database migration. - Minted human-only `request_confirmation` cards with server-owned `secretProposal` metadata. - Rejected agent-supplied governed metadata and agent addressees. - Rechecked `agent_config:update` authority under the proposal lock before execution. - Recorded `executed` or `failed` results and posted a failure comment when no binding was created. - Settled failed accepted proposals atomically and mirrored rejection, withdrawal, and expiry in both directions. - Emitted `secret.binding.created` for new agent binding writes. - Added a dedicated issue-thread card for pending, executed, failed, rejected, withdrawn, and expired states. - Showed only the source label, target agent, config path, skeptical justification, expiry, and safe failure code. - Replaced resolved attention-query entries immediately with the stitched server result. - Added focused server, database, UI, and state-transition tests. - Added Storybook fixtures for every review state and documented the API and agent behavior. ## Verification - `pnpm exec vitest run ui/src/components/IssueThreadInteractionCard.test.tsx ui/src/components/AttentionInteractionResolver.test.ts` — 58 passed. - `pnpm --filter @paperclipai/ui typecheck` - `pnpm check:token-gates` - `pnpm build-storybook` - `pnpm --filter @paperclipai/shared typecheck` - `pnpm --filter @paperclipai/db typecheck` - `pnpm --filter @paperclipai/server typecheck` - `pnpm --filter @paperclipai/db check:migrations` - `NODE_ENV=test pnpm exec vitest run server/src/__tests__/issue-thread-interaction-routes.test.ts server/src/__tests__/secret-proposals-routes.test.ts server/src/__tests__/secrets-routes.test.ts server/src/__tests__/agents-service-secret-bindings.test.ts` — 142 passed. - `NODE_ENV=test pnpm --filter @paperclipai/db exec vitest run src/company-secret-proposals-migration.test.ts --silent` — 1 passed. - `pnpm -r typecheck` - `pnpm test:run` — server 4,175 passed, UI 4,109 passed; the CLI AWS-doctor case passes 8/8 with runtime-injected static AWS credential variables unset. - `pnpm build` - `git diff --check origin/master...HEAD` ## Risks - Migration `0221` adds one nullable foreign key and one index. It uses idempotent guards. - The accept route performs a governed write after it records card acceptance. A failed write is visible and settles the proposal as rejected. - Concurrent proposal and card resolution must use proposal-before-interaction lock order. A race test covers direct approval against card rejection. - The new audit event increases activity rows for newly added agent bindings. It does not include secret values or fingerprints. - The card includes only safe proposal metadata. It does not include secret value, fingerprint, version, or internal secret identifiers. - The UI uses the stitched resolution result. Focused tests cover immediate cache replacement and every terminal state. > This work extends an existing completed roadmap capability. The GitHub duplicate search returned no other open related work. ## Model Used - OpenAI Codex with model ID `gpt-5`. The runtime did not expose its context-window size. Reasoning, repository tools, code execution, database integration tests, UI rendering, and GitHub tools were enabled. ## 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> |
||
|
|
0819cac4c6 |
feat(secrets): add agent-readable /secrets/catalog endpoint (#9530)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Agents can be configured with env bindings that reference company
secrets — they specify which secret by UUID in `adapterConfig.env`
> - But there is no API endpoint agents can call to look up a secret
UUID by name — `GET /companies/:companyId/secrets` is board-only, and
the internal `secrets.resolve` handler only accepts UUIDs
> - So when an agent needs to wire a new secret (e.g. an API key for a
new skill), it has no way to discover the UUID from a known name like
`HOMEBOX_API_KEY` — the user must find it by inspecting browser network
traffic
> - The fix is a read-only catalog endpoint that agents can call to get
the `id`/`name`/`key`/`status` mapping — no values, no provider config —
just enough to resolve a name to a UUID
> - This PR adds `GET /companies/:companyId/secrets/catalog`, guarded by
`assertBoardOrAgent` + `assertCompanyAccess`, so agents can discover the
UUID they need without board-level access and without any secret value
being exposed
## Linked Issues or Issue Description
No pre-existing public issue. Describing inline per the feature request
template:
**Subsystem affected:** `server/` — REST API & orchestration services
**Problem or motivation:**
Agents that configure env bindings must reference secrets by UUID
(`secretId`). There is no agent-accessible API to resolve a secret name
to its UUID. `GET /companies/:companyId/secrets` requires board access;
the internal `secrets.resolve` handler rejects anything that is not
already a UUID. Agents and their operators are forced to find UUIDs by
inspecting browser network requests, which is friction that should not
exist.
**Proposed solution:**
Add a read-only catalog endpoint — `GET
/companies/:companyId/secrets/catalog` — that agents can call. It
returns only non-sensitive metadata (`id`, `name`, `key`, `status`) for
each active company secret, stripped of values, provider configuration,
and version history. Board callers get the same response. The existing
full-detail list endpoint (`GET /companies/:companyId/secrets`) remains
board-only and is unchanged.
**Alternatives considered:**
- Allow agents to call the existing `/secrets` list — rejected because
it returns full rows including provider metadata; narrowing the response
is safer.
- Add a name-to-UUID lookup by query param — simpler but less useful; a
full catalog means the agent can do the resolution locally without a
second round-trip.
**Roadmap alignment:** Does not duplicate anything in `ROADMAP.md`.
## What Changed
- `server/src/routes/secrets.ts` — new `GET
/companies/:companyId/secrets/catalog` route registered before the
board-only `GET /companies/:companyId/secrets` route. Uses
`assertBoardOrAgent` + `assertCompanyAccess`. Calls `svc.list()` then
projects each row to `{ id, name, key, status }` before responding.
- `server/src/__tests__/secrets-routes.test.ts` — adds `list` to the
shared mock service object (it was missing); adds a `describe` block
with four test cases: board caller receives stripped metadata, agent
caller in the same company receives stripped metadata, unauthenticated
request gets 401, agent from a different company gets 403.
## Verification
**Automated:**
```bash
pnpm --filter @paperclipai/server test --run secrets-routes
```
All four new test cases (board access, agent access, unauthed rejection,
cross-company rejection) should pass.
**Manual:**
1. Start the Paperclip server locally.
2. Create a company and a secret via the UI.
3. Call the endpoint as a board user:
```bash
curl http://localhost:3100/api/companies/<companyId>/secrets/catalog \
-H "Authorization: Bearer <board-session-token>"
```
Expect a JSON array with `id`, `name`, `key`, `status` fields — no
`provider`, no `referenceCount`, no version data.
4. Call the same endpoint with an agent API key:
```bash
curl http://localhost:3100/api/companies/<companyId>/secrets/catalog \
-H "Authorization: Bearer <agent-api-key>"
```
Expect the same response.
5. Call with an agent API key scoped to a *different* company — expect
403.
## Risks
Low risk. This is a purely additive, read-only endpoint. No existing
behavior changes. The only new capability is that agents can discover
the UUIDs of secrets in their own company — metadata they already need
to do their job. Secret values are never returned. Authorization reuses
the existing `assertBoardOrAgent` and `assertCompanyAccess` guards
already used throughout the codebase.
## Model Used
Claude Sonnet 4.6 (`claude-sonnet-4-6`) — Anthropic, extended context,
tool use enabled. The entire change (route, tests, PR description) was
produced by the model operating as a Paperclip CEO agent assigned to the
task.
## 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
- [ ] All Paperclip CI gates are green
- [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups
- [ ] I will address all Greptile and reviewer comments before
requesting merge
---------
Co-authored-by: Austin Pilz <austinpilz@users.noreply.github.com>
Co-authored-by: root <root@paperclip.pilz.dev>
Co-authored-by: Internet Historian <agent@paperclip.internal>
Co-authored-by: Andrew Aymeloglu <aaymeloglu@gmail.com>
|
||
|
|
0b496c9c03 |
feat(secrets): add run-bound agent secret access (#9921)
## Thinking Path > - Paperclip is the open source control plane people use to manage AI-agent companies. > - Agents already receive selected company secrets through `env.*` bindings at run launch, but environment injection is ambient, long-lived, and not suitable for every secret consumer. > - The existing binding and secret-access-event models already provide company-scoped authorization and per-resolution audit seams. > - Agents need an explicit way to discover only the secrets granted to them and fetch a value on demand without exposing the wider company catalog. > - That capability must remain run-bound, preserve low-trust token carve-outs, and make every value read visible in both security and operator audit trails. > - This pull request adds an `access.*` delivery namespace, two run-bound agent routes, dual audit logging, documentation, and an operator grants editor. > - The benefit is least-privilege, revocable, auditable secret access while preserving existing env injection behavior. ## Linked Issues or Issue Description No pre-existing public issue. Related work: - Refs #9797 — existing in-sheet agent access UI that this PR extends to distinguish env and API delivery. - Refs #9918 — complementary searchable-agent picker improvement for the same secrets sheet. - Refs #9530 — related company-wide metadata catalog proposal; this PR intentionally exposes only the authenticated run's granted aliases and values. **Problem / motivation:** Agents can currently consume secrets only through process environment injection. This keeps values resident for the run, does not support on-demand consumers, and cannot provide a discrete operator-visible activity event for each agent-initiated read. **Proposed solution:** Treat `company_secret_bindings` as the source of truth for agent secret grants. Keep `env.KEY` as env delivery and add `access.ALIAS` for API-only delivery; an env binding also implies read access because the value is already present in the agent process. Add run-bound list/fetch endpoints that derive scope from the authenticated heartbeat run and never accept caller-selected overlays. **Alternatives considered:** A company-wide agent-readable catalog was rejected for this value path because it increases reconnaissance and does not prove a per-secret grant. Reusing the ephemeral environment-probe resolver was rejected because it lacks binding enforcement. Approval-gated reads and user-scoped secrets remain deferred beyond v1. **Roadmap alignment:** This extends the completed **Secrets Manager with per-agent access** roadmap capability from launch-time env injection to explicit run-bound API delivery without duplicating a separate planned initiative. ## What Changed - Added `access.*` agent binding validation and a dedicated run-bound resolver that combines `secrets:read` authorization with binding-context enforcement. - Added `GET /api/agents/me/secrets` for minimal granted metadata and `POST /api/agents/me/secrets/:key/value` for on-demand value fetches with `Cache-Control: no-store`. - Preserved the existing denials for low-trust review agents, task-bridge credentials, and skill-test tokens; standard long-lived agent API keys cannot call the run-bound routes. - Added dual audit behavior: value attempts write `secret_access_events` and `activity_log` (`secret.value.read`), while metadata listing writes the lighter `secret.access.listed` activity event. - Kept env compatibility: `env.*` remains injected at launch and also implies API read for the same bound agent; `access.*` never becomes an environment variable. - Added the agent-settings **Secret access** editor plus delivery-mode/alias surfacing on the Secrets page, with focused UI tests and tokenized layout styles. - Updated OpenAPI, shared types, agent-facing skill documentation, and API reference documentation. ### UI Screenshots P3 produced and reviewed three screenshots using mock data; images are intentionally not committed to the repository: - `secret-access-editor.png` — agent settings grant editor. - `secret-access-light.png` — Secrets-page delivery surfacing in light mode. - `secret-access-dark.png` — Secrets-page delivery surfacing in dark mode. The source attachments are retained with the implementation task and linked in the internal handoff; the public page publisher was unavailable in the PR-prep runtime. ## Verification - `pnpm exec vitest run server/src/__tests__/agent-secrets-routes.test.ts server/src/__tests__/secrets-service.test.ts server/src/__tests__/secrets-routes.test.ts ui/src/lib/secret-delivery.test.ts ui/src/components/AgentSecretAccessEditor.test.tsx` — 5 files, 122 tests passed. - Security follow-up: `pnpm exec vitest run server/src/__tests__/agent-secrets-routes.test.ts server/src/__tests__/secrets-service.test.ts` — 2 files, 73 tests passed after active-run and version-consistency fixes. - Final-head CI: all feature, typecheck, build, e2e, security, and review gates pass; `General tests (server (1/3))` remains red after one rerun because unrelated `heartbeat-retry-scheduling.test.ts` cleanup deletes `heartbeat_runs` before referenced `activity_log` rows. - `pnpm --filter @paperclipai/server typecheck` — passed. - `pnpm --filter @paperclipai/shared typecheck` — passed. - `pnpm --filter @paperclipai/ui typecheck` — passed. - `pnpm check:token-gates` — feature-local arbitrary-value violations fixed; command still reports five unchanged `#9627` literals outside this PR. - End-to-end QA passed all eight acceptance criteria: grant/list, fetch, dual audit, env-implies-read, denial matrix, revocation, UI rendering, and env-injection regression. Evidence: https://github.com/paperclipai/paperclip/pull/9921#issuecomment-5027455492 - Security review returned PASS-with-required-changes; the implementation uses the required dedicated binding-enforcing resolver, run-bound JWT restriction, run-derived overlays, minimal metadata, and a resolver redaction-registration hook. Evidence: https://github.com/paperclipai/paperclip/pull/9921#issuecomment-5027455382 ## Risks - A compromised agent can exfiltrate any secret explicitly granted to it; explicit company-scoped/run-scoped grants, revocation, and audit reduce but cannot remove that inherent capability risk. - The resolver invokes a redaction-registration hook before returning values, but the current route has no persistent cross-request per-run redaction registry. Paperclip-owned later comments/events therefore cannot yet guarantee automatic scrubbing of a deliberately copied fetched value; QA classified this as non-blocking residual hardening. - Audit-event insertion currently fails open if the security-event insert itself fails; the operator activity event provides partial redundancy, but a future hardening change should define fail-closed behavior for value delivery. - This PR overlaps `ui/src/pages/Secrets.tsx` with #9918 and may require a straightforward rebase after that PR moves. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, `gpt-5.3-codex`, with reasoning, repository tool use, terminal execution, Paperclip API access, and GitHub CLI capabilities. Context-window size is not exposed by the runtime. - Anthropic Claude Opus 4.8 with 1M context and tool use assisted with the UI implementation commit. ## 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> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
Jannes StubbemannandClaude Opus 4.7
|
7f2ed0ad90 |
security(server): close cross-tenant existence oracle (404 instead of 403) (#3967)
## Thinking Path > - Paperclip orchestrates AI agents for zero-human companies > - In a multi-tenant deployment, route handlers that take a resource id (`issue`, `goal`, `project`, `approval`, etc.) look the resource up by id and then call `assertCompanyAccess` on its `companyId` — 404 if it doesn't exist, 403 if it exists in another tenant > - The split status codes are a classic *existence oracle*: any authenticated user can enumerate ids across tenants by probing for the 403/404 boundary, mapping out which issues, labels, approvals, etc. exist in other customers' tenants even when they cannot read the contents > - The right fix is a single uniform 404 for both "not found" and "found but cross-tenant", which collapses the oracle but still preserves write-path checks (active membership, viewer-readonly) for *authorized* tenants > - This pull request adds a non-throwing `hasCompanyAccess(req, companyId)` helper plus a `getAccessibleResource` wrapper that ~130 handlers across 14 route files now use, folding the access check into the existence check while still running `assertCompanyAccess` for authorized tenants so viewer-readonly / inactive-membership rejections fire unchanged on write paths > - The benefit is closing a multi-tenant information leak without breaking write-path security or single-tenant local-first behavior ## Linked Issues or Issue Description Refs #709 — asks for company-scope regression coverage across approval/activity/access routes, because a subtle route refactor could leak cross-tenant data; this PR hardens exactly those surfaces (uniform 404 across 14 route files including `approvals`, `activity`, `secrets`) and updates cross-tenant expectations in test files. It does not add the full coverage matrix #709 asks for — hence Refs, not Closes. No existing issue covers the oracle itself — described in-PR: - Route handlers returned 404 for "not found" but 403 for "exists in another tenant", a classic *existence oracle*: any authenticated user could enumerate ids across tenants by probing the 403/404 boundary. - That maps out which issues, labels, approvals, etc. exist in other customers' tenants even when their contents are unreadable. - Fix: a uniform 404 for both cases, while keeping write-path checks (active membership, viewer-readonly) for authorized tenants. ## What Changed - **`server/src/routes/authz.ts`** — new `hasCompanyAccess(req, companyId): boolean` helper alongside the existing `assertCompanyAccess`. Docstring spells out the two-step pattern (404 gate, then `assertCompanyAccess` for write-path checks). The helper mirrors `assertCompanyAccess`'s company-scope semantics exactly — in particular, signed-in instance admins do **not** get blanket access to companies they are not a member of (the repo's `authz-company-access` tests pin that behavior for `assertCompanyAccess`; an earlier draft of the helper accidentally widened it for reads). - **`getAccessibleResource(req, res, lookup, notFoundMessage)`** — the safe thing is now the easy thing. One helper wraps the whole pattern (uniform 404 for missing/cross-tenant, then `assertCompanyAccess` for write-path membership checks) and ~130 handlers across 14 route files use it: ```ts const goal = await getAccessibleResource(req, res, svc.getById(id), "Goal not found"); if (!goal) return; ``` Files: `activity`, `agents`, `approvals`, `assets`, `costs`, `environments`, `execution-workspaces`, `file-resources`, `goals`, `issue-tree-control`, `issues`, `projects`, `routines`, `secrets`. Handlers with bespoke not-found behavior (the legacy `200 []` contract, audit-logged denials in `file-resources`, null-returning authz helpers) compose `hasCompanyAccess` directly using the documented two-step pattern: ```ts // step 1: close the oracle (uniform 404 for both not-found and cross-tenant) if (!existing || !hasCompanyAccess(req, existing.companyId)) { res.status(404).json({ error: "Goal not found" }); return; } // step 2: enforce write-path membership checks for authorised tenants (no-op on GET) assertCompanyAccess(req, existing.companyId); ``` Routes where `companyId` comes from *request input* (`req.params.companyId`, `req.body.companyId`, e.g. in `companies.ts` and `plugins.ts`) deliberately retain plain `assertCompanyAccess` — there's no existence oracle to close because the companyId is an input, not a discovered value. - **Full-sweep coverage** — a scripted audit of every `assertCompanyAccess(req, <resource>.companyId)` call site in `server/src/routes/` found ~55 lookup-then-assert pairs the first pass missed; all are now gated. Notable ones: the `/secret-provider-configs/:id` CRUD routes, the agents instructions-bundle/config-revision/skills-sync routes (which check access via the `assertCanUpdateAgent` / `assertCanReadAgent` / `assertCanManageInstructionsPath` helpers), `POST /heartbeat-runs/:runId/watchdog-decisions`, `GET /issues/:id/cost-summary`, the environment + environment-lease GET routes, all six issue-tree-control routes, ~24 issue sub-resource routes (document annotations, interactions, approvals links, recovery actions, plan decompositions, lock/unlock), and the three workspace file-resource routes (these throw `notFound` instead of `forbidden` inside their audit-logging wrappers, so denied attempts are still activity-logged server-side while the client sees a uniform 404). - **Helpers made self-defending** — `assertCanUpdateAgent` / `assertCanReadAgent` / `assertCanManageInstructionsPath` (agents) and `assertCanManage{Project,Execution}WorkspaceRuntimeServices` throw `notFound` for cross-tenant resources before their `assertCompanyAccess` step, so a future caller that forgets the route-level gate still can't reopen the oracle. - **Pattern enforcement** — new `authz-existence-oracle-guard.test.ts` statically scans `server/src/routes/*.ts` and fails CI on any `assertCompanyAccess(req, <resource>.companyId)` call that is not preceded by a `hasCompanyAccess` gate, with an explicit allowlist (plus staleness check) for the request-input cases. New routes that regress to the 403/404 split fail the suite with a message pointing at the documented pattern. - **Tests** — cross-tenant expectations updated from 403→404 where routes are now gated; new `hasCompanyAccess` unit tests in `authz-company-access.test.ts` pin the instance-admin/local-implicit/agent/none semantics in lockstep with `assertCompanyAccess`; `write-path-membership.test.ts` (added in an earlier round) confirms viewer/inactive users are still rejected on writes. - **One legacy-contract preserve** — `GET /heartbeat-runs/:runId/issues` still returns `200 []` for both "doesn't exist" and "cross-tenant" so the legacy contract is preserved while the oracle stays closed. ## Verification - `pnpm run typecheck` — PASS. - `pnpm -F @paperclipai/server exec vitest run` — full server suite green locally apart from 4 pre-existing local-environment failures (`paperclip-skill-utils` ×2 and `workspace-runtime` ×1 are cwd/git-environment dependent — verified identical on a clean checkout of the base; `heartbeat-process-recovery` is the known macOS flake). - The new `authz-existence-oracle-guard` test sweeps `server/src/routes/*.ts` and confirms no remaining `assertCompanyAccess(resource.companyId)` site without a `hasCompanyAccess` gate; the only allowlisted holdouts take `companyId` from request input. ## Risks - **API contract narrowing.** Any client that specifically checked for `403` on cross-tenant access now sees `404`. This is a strict narrowing (one status instead of two for the same negative outcome) and matches what a client should expect for any id it can't access. - **Write-path checks preserved.** `assertCompanyAccess` still runs after the 404 gate on write routes, so viewer-readonly / inactive-membership rejections fire unchanged for legitimate users. - **Instance-admin scope unchanged.** `hasCompanyAccess` denies signed-in instance admins without an explicit membership, exactly like `assertCompanyAccess` (pinned by unit tests) — so the gate introduces no new read access for admins. - **Single-tenant local-first deploys** behave identically — the helper short-circuits to `true` for `local_implicit` sessions. - No new env vars, no deployment-mode switch. ## Model Used Claude Opus 4.7 (1M context), extended thinking mode; completeness sweep + instance-admin parity fix by Claude Fable 5 (1M context). ## Checklist - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] Thinking path traces from project context to this change - [x] Model used specified - [x] Checked ROADMAP.md — part of the multi-tenant hardening initiative - [x] Tests run locally and pass - [x] Added/updated cross-tenant 404 expectations across test files - [x] No UI changes - [x] Documented risks above - [x] Will address all Greptile and reviewer comments before merge Part of the multi-tenant hardening initiative — see also #5864 (per-company JWT keys) and #5865 (plugin tables `company_id`). --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
09f503f216 |
[codex] Surface AWS secret provider create errors (#9161)
Preserve sanitized AWS Secrets Manager create errors and make rollback/cleanup failures explicit. Co-Authored-By: Paperclip <noreply@paperclip.ing> |
||
|
|
ad961227f5 |
feat(secrets): add user-specific runtime secrets (#8825)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent runs often need provider credentials, API tokens, and other environment-bound secrets. > - Company-level secrets work for shared credentials, but they do not model values that should differ by human operator. > - Without a user-scoped model, a run can dispatch without knowing whether the responsible human has supplied the needed value. > - Paperclip also needs run attribution to make those user-scoped runtime checks deterministic and auditable. > - This pull request adds user-specific secret definitions, per-user values, environment bindings, responsible-user attribution, and runtime resolution gates. > - The benefit is that teams can define the secret once, let each user provide their own value, and block runs before dispatch when required user secrets or active definitions are unavailable. ## Linked Issues or Issue Description Refs #224 Refs #6057 This PR implements user-specific secret support as a core secret-management capability rather than a one-off adapter setting. It is related to existing public work on company secrets UI and runtime secret refs, but is distinct because the value is owned by the responsible user and resolved at run dispatch time. Related PR search before opening found existing secrets work such as #1550, #8256, #8614, #8634, and #8647; none of those add the full user-secret definition/value/runtime gate covered here. ## What Changed - Added user-secret definitions and per-user "My secrets" values, keeping stored values out of access metadata. - Added `user_secret_ref` environment bindings and UI affordances to pick them alongside existing secret refs. - Added responsible-user runtime resolution so user-secret refs resolve against the human responsible for the run. - Added pre-dispatch missing-secret gates so runs fail before adapter dispatch when required user values are absent or definitions are inactive. - Added low-trust allowlist hardening for user-secret runtime access. - Added issue, routine, run, and agent API key responsible-user attribution and fail-closed dispatch behavior when attribution cannot be resolved. - Added denial-copy mapping so responsible-user authorization failures surface as actionable run outcomes instead of opaque setup failures. - Added OpenAPI documentation for the user-secret routes. - Rebases cleanly on current `master`; migrations were renumbered incrementally as `0128_user_specific_secrets`, `0129_agent_api_key_responsible_user`, and `0130_run_responsible_user_invariant` after upstream `0126`/`0127` migrations. - Removed previously committed local design screenshots so the PR contains code/docs/tests only. ## Verification - PASS: PR head `2527febd106bcf3ca264ca0da7fca491084192d6` is based on `paperclipai/paperclip:master`. - PASS: `git diff --check` - PASS: `git diff --name-only public/master...HEAD | rg '^(pnpm-lock\\.yaml|\\.github/workflows/|screenshots/)' || true` produced no files. - PASS: migration journal audit confirmed unique indexes through `130` with tail entries `0126_issue_comment_derived_attribution`, `0127_environment_custom_images_instance_scoped`, `0128_user_specific_secrets`, `0129_agent_api_key_responsible_user`, and `0130_run_responsible_user_invariant`. - PASS: `pnpm --filter @paperclipai/ui typecheck` - PASS: `pnpm --filter @paperclipai/server typecheck` - PASS: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-responsible-user-invariant.test.ts` - PASS: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-active-run-output-watchdog.test.ts src/__tests__/heartbeat-stale-queue-invalidation.test.ts src/__tests__/heartbeat-workspace-finalize-branch.test.ts src/__tests__/issue-monitor-scheduler.test.ts` - PASS: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-comment-wake-batching.test.ts src/__tests__/heartbeat-retry-scheduling.test.ts src/__tests__/heartbeat-accepted-plan-workspace-refresh.test.ts src/__tests__/heartbeat-plugin-environment.test.ts` - PASS: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/low-trust-red-team-routes.test.ts` - PASS: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/secrets-service.test.ts` (55 tests) - PASS: `pnpm vitest run server/src/__tests__/secrets-routes.test.ts server/src/__tests__/secrets-service.test.ts` (89 tests after final Greptile cleanup fixes) - PASS: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/heartbeat-issue-liveness-escalation.test.ts` (17 tests after the final rebase CI fix) - PASS: focused server Vitest batches covering heartbeat recovery, project env, plugin env, routines, low-trust, pipelines, monitors, watchdog, and stale queue paths. - PASS: GitHub checks are green on `2527febd106bcf3ca264ca0da7fca491084192d6`, including Typecheck + Release Registry, Build, General tests, serialized server suites, e2e, Canary Dry Run, verify, security checks, and Greptile Review. - PASS: Greptile Review completed successfully on `2527febd106bcf3ca264ca0da7fca491084192d6` with Confidence Score 5/5, and GraphQL review-thread audit returned zero unresolved non-outdated threads. ## Risks - Runtime behavior now depends on a run having a correct responsible user; missing or incorrect responsibility assignment can block runs before adapter dispatch. - `user_secret_ref` bindings intentionally expose metadata without values, but UI/API callers may need to handle the new binding kind explicitly. - External secret providers and IAM policies are not automatically provisioned by this PR; operators still need to configure provider-side access for non-local vaults. - The PR is broad across db/shared/server/UI/runtime paths, so release validation should include both API and UI secret workflows before merge. - The migration renumbering is intentionally incremental after upstream migrations; the branch migrations use guarded column/table/index/constraint creation so users who tested the older draft numbering should not hit duplicate DDL for the existing objects. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex, GPT-5-based coding agent (`gpt-5`), Codex local adapter with shell/tool use and code execution. Context window and internal reasoning mode are not exposed by the runtime. ## 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> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
DottaandPaperclip
|
43b005b704 |
Add pipeline workflow primitives and operator UI (#7903)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The pipeline subsystem models repeatable work as items moving through stages, with agent automation, review gates, blockers, drift notices, and linked work. > - Operators need this to be usable as one coherent workflow surface, not just as backend primitives or disconnected route experiments. > - The branch now carries the pipeline data model, service/routes, CLI/tutorial path, aggregation feeds, operator UI, stage automation controls, liveness/retry handling, and follow-up polish that make the primitive reviewable end to end. > - This pull request is the single review target for that pipeline workflow primitive stack. > - The benefit is that reviewers can evaluate the full operator experience and server contract together against `master`. ## Linked Issues or Issue Description No public GitHub issue exists for this work. The underlying feature request is described inline. ### Problem or motivation Paperclip needs a first-class way to model multi-stage agent/company workflows where upstream items can spawn downstream work, request review, carry fields across pipelines, surface drift, retry automation, and show operators where work is blocked or active. Without a unified pipeline primitive, these workflows spread across ad hoc issues, routines, and comments, making the state hard to inspect or operate. ### Proposed solution Add the pipeline workflow primitive stack: database schema and migrations, shared validators/types, server services and REST routes, aggregation and liveness helpers, CLI/tutorial smoke support, and the React operator UI for pipeline lists, boards, item detail, review/learnings views, settings, stage automation, secrets, carry-over fields, and retry/recovery flows. ### Alternatives considered - Keep workflows as loosely linked issues and routines: rejected because operators need a single board/detail/settings surface for repeated workflow patterns. - Ship backend primitives first and defer UI: rejected for this branch because the operator experience is the main way to validate the primitive. - Add a narrower one-off content workflow: rejected because the same primitives are useful across future company processes. ## What Changed - Added and evolved pipeline schema, migrations, shared contracts, server services, REST routes, route tests, and CLI/tutorial smoke support. - Added pipeline aggregation, health/liveness, drift acknowledgment, blocker/carry-over, automation retry, stage automation environment, and permission recovery behavior. - Added the operator UI for pipeline index/board/item detail/settings/review/learnings flows, including stage secrets, automation controls, markdown/item descriptions, linked issue assets, liveness banners, and source automation metadata. - Refactored issue document frame rendering through the shared `DocumentFrameHeader` component to keep document controls consistent with the pipeline document surfaces. - Kept this PR as the single base-branch review target for the current pipeline branch. ## Verification Current branch refresh: - `pnpm vitest run server/src/__tests__/pipelines-service.test.ts` — 31 passed - `pnpm vitest run server/src/__tests__/pipelines-routes.test.ts` — 19 passed - `pnpm --filter ./server typecheck` — passed - `pnpm --filter ./ui typecheck` — passed - Verified Pipelines remains gated by `enablePipelines === true`: sidebar item is hidden unless the flag is enabled, direct pipeline routes redirect to `/dashboard` when disabled, and the Experimental settings UI still has no Pipelines toggle. - GitHub status checks on `df071c710646de625131064c3fb6588b5e97964a` — all complete with no failing conclusions, including Actions, Socket, Superagent/Security, and Greptile Review - Greptile summary on `df071c710646de625131064c3fb6588b5e97964a` — Confidence Score 5/5 - GitHub review-thread sweep — 0 unresolved Greptile threads Previously recorded during branch development: - Server pipeline service/route and aggregation tests - Shared validator tests - UI pipeline page/settings/item-detail/learnings/liveness tests - Pipeline tutorial smoke path ## Risks - High review surface: this is a large feature branch spanning database, shared contracts, server behavior, CLI/docs, and UI. - Migration ordering and schema compatibility need reviewer attention because this branch has been kept current across multiple `master` syncs. - GitHub still reports merge state `BLOCKED` because the PR is awaiting normal human review/branch-protection completion; all current status checks are green. - Branch-name checklist exception: this PR uses the pre-existing requested branch name, which predates the current public-branch naming rule. The PR title/body avoid internal issue references. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used OpenAI Codex coding agent based on GPT-5, with repository tool use, shell execution, git/GitHub CLI operations, and local verification commands. Earlier commits in this branch were assisted by Paperclip agents and other AI coding agents as recorded in commit authorship. ## 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) - [ ] 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> |
||
|
DottaandPaperclip
|
f88ac9d078 |
[codex] Fix local skill, secrets, and file viewer regressions (#8586)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agents rely on local skills, provider-backed secrets, and workspace file previews during normal execution. > - Local skill imports need bounded reference-file inventory so direct skill discovery stays accurate without accidentally walking too much of the filesystem. > - Secrets provider setup needs actionable AWS discovery errors so operators can recover from IAM/config problems without losing manual form input. > - The issue detail file viewer should reopen cleanly after the first close so users can keep inspecting files during task review. > - This pull request collects the small fixes and regression tests for those related operator workflows. > - The benefit is more predictable local skill imports, clearer secrets setup failure states, and a less brittle file preview interaction. ## Linked Issues or Issue Description - No public GitHub issue was found for this extracted local work. - Related prior sync context: #8536. - Problem: local skill reference discovery, AWS provider-vault discovery errors, and issue file preview reopening each had narrow workflow regressions that made operator recovery harder. - Expected behavior: skill imports inventory reference files within bounded local skill directories, AWS discovery failures present safe actionable guidance while preserving manual values, and closing the first file preview does not prevent opening another preview. - Reproduction scope: import a local skill with referenced files, attempt AWS Secrets Manager discovery with insufficient IAM/list permissions, and open/close/reopen file previews from an issue detail page. - Duplicate search: searched GitHub PRs/issues for `skill inventory secrets file viewer` and `skill inventory secrets AWS file viewer`; no matching public duplicate was found. ## What Changed - Bounded direct local skill file inventory discovery and added regression coverage for reference file imports. - Preserved and surfaced safe, actionable AWS Secrets Manager discovery/import errors in server responses and the secrets UI. - Kept AWS provider-vault manual form values intact when discovery fails or returns no candidates. - Fixed issue file viewer state so closing the first preview still allows later file previews to open. - Updated the secrets render test harness to avoid the missing `React.act` export in the current React package set. ## Verification - `pnpm run preflight:workspace-links && pnpm exec vitest run server/src/__tests__/company-skills-service.test.ts server/src/__tests__/secrets-routes.test.ts server/src/__tests__/secrets-service.test.ts ui/src/context/FileViewerContext.test.ts ui/src/pages/Secrets.render.test.tsx` - Result: 5 test files passed, 116 tests passed. - Install note: the isolated worktree needed `NODE_ENV=development pnpm install --frozen-lockfile --prod=false --force` before local verification because it initially had no dev dependencies installed. ## Risks - Low-to-medium risk: this touches skill import inventory, secrets-provider error handling, and file-viewer UI state, but each change is covered by focused regression tests. - No migrations. - No dependency or lockfile changes. - CI is rerunning on the latest head after review fixes; Greptile is 5/5 with no unresolved Greptile threads. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex coding agent, GPT-5-family model as provided in the Paperclip run environment, with repository tool use and local command execution. ## 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> |
||
|
DottaandPaperclip
|
d67347be77 |
[codex] Provider vault secrets UX (#6381)
## Thinking Path > - Paperclip orchestrates AI agents that need scoped, auditable access to secrets > - Hosted and external deployments need provider vault configuration without exposing secret values in Paperclip metadata > - AWS Secrets Manager vault setup previously required too much manual operator knowledge > - Provider vault discovery and removal belong together as an independent secrets-management improvement > - This pull request adds AWS provider vault discovery/prefill plus vault removal flows > - The benefit is a safer operator path for configuring external secret storage before higher-level cloud workflows depend on it ## What Changed - Added shared validators/types for AWS provider vault discovery payloads and safe provider metadata. - Implemented AWS provider vault discovery preview on the server. - Added provider vault removal service/route behavior. - Added Secrets page UI for discovery prefill, removal messaging, and related rendering coverage. - Added Storybook provider-vault fixtures and captured screenshots for the new UX states. ## Verification - `pnpm install --frozen-lockfile --ignore-scripts` - `pnpm exec vitest run packages/shared/src/validators/secret.test.ts server/src/__tests__/aws-secrets-manager-provider.test.ts server/src/__tests__/secrets-routes.test.ts server/src/__tests__/secrets-service.test.ts ui/src/pages/Secrets.render.test.tsx` - Result: 4 files passed, 1 embedded Postgres-backed file skipped on this host because local Postgres init was unavailable. - `pnpm --filter @paperclipai/ui exec vitest run src/pages/Secrets.render.test.tsx` - `pnpm --filter @paperclipai/ui typecheck` - Storybook screenshot capture against `Product/Secrets` on `http://127.0.0.1:60381/iframe.html?id=product-secrets--secrets-inventory&viewMode=story&globals=theme:dark` ## Screenshots Provider vaults tab after this change:  AWS discovery candidate flow:  Provider vault removal confirmation:  ## Risks - Secret provider metadata handling must remain non-sensitive; validators reject credential-bearing Vault URLs and sensitive AWS discovery keys. - AWS discovery depends on deployment credentials being configured correctly outside Paperclip-managed company secrets. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5-based coding agent with local shell/git/tool use. Exact hosted model ID and context-window size are not exposed by the local Paperclip adapter runtime. ## 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 run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> |
||
|
|
778e775c35 |
Add secrets provider vaults and remote import (#5429)
## Thinking Path > - Paperclip orchestrates AI-agent companies and needs secrets handling to work across local development, hosted operators, and governed agent execution. > - The affected subsystem is the company-scoped secrets control plane: database schema, server services/routes, CLI workflows, and the Secrets settings UI. > - The gap was that secrets were local-only and operators could not manage provider vaults or import existing remote references without exposing plaintext. > - This branch adds provider vault configuration plus an AWS Secrets Manager remote-import path while preserving company boundaries, binding context, and audit trails. > - I kept the PR to a single branch PR, removed unrelated lockfile/package drift, rebased the full branch onto the current `public-gh/master`, and addressed fresh Greptile findings. > - The benefit is a reviewable implementation of provider-backed secrets with focused tests covering provider selection, import conflicts, deleted secret reuse, rotation guards, and AWS signing behavior. ## What Changed - Added provider vault support for company secrets, including provider config storage, default vault handling, health checks, binding usage, access events, and remote import preview/commit. - Added an AWS Secrets Manager provider using SigV4 request signing, bounded request timeouts, namespace guardrails, cached runtime credential resolution, and external-reference linking without plaintext reads. - Added Secrets UI surfaces for vault management and remote import, plus CLI/API documentation for setup and operations. - Stabilized routine webhook secret binding paths and SSH environment-driver fixture bindings discovered during verification. - Addressed Greptile and CI findings: no lockfile/package drift, monotonic migration metadata, disabled-vault default races, soft-deleted secret hiding/recreate behavior, remove behavior with disabled vaults, soft-deleted external-reference re-import, non-active rotation guards, managed-secret soft deletion through PATCH, and per-call AWS SDK credential client churn. - Rebased this branch onto `public-gh/master` at `0e1a5828` and force-pushed with lease to keep this as the single PR for the branch. ## Verification - `git fetch public-gh master` - `git rebase public-gh/master` - `git diff --name-only public-gh/master...HEAD | grep '^pnpm-lock\.yaml$' || true` confirmed `pnpm-lock.yaml` is not in the PR diff. - Confirmed migration ordering: master ends at `0081_optimal_dormammu`; this PR adds `0082_dry_vision` and `0083_company_secret_provider_configs`. - Inspected migrations for repeat safety: new tables/indexes use `IF NOT EXISTS`; foreign keys are guarded by `DO $$ ... IF NOT EXISTS`; column additions use `ADD COLUMN IF NOT EXISTS`. - `pnpm -r typecheck` passed before the Greptile follow-up commits. - `pnpm test:run` ran the full stable Vitest path before the Greptile follow-up commits; it completed with 3 timing-related failures under parallel load: `codex-local-execute.test.ts`, `cursor-local-execute.test.ts`, and `environment-service.test.ts`. - `pnpm --filter @paperclipai/server exec vitest run src/__tests__/codex-local-execute.test.ts src/__tests__/cursor-local-execute.test.ts src/__tests__/environment-service.test.ts` passed on targeted rerun (`24/24`). - `pnpm build` passed before the Greptile follow-up commits. Vite reported existing chunk-size/dynamic-import warnings. - After Greptile follow-up commits: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/secrets-service.test.ts` passed (`26/26`). - After Greptile follow-up commits: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/aws-secrets-manager-provider.test.ts src/__tests__/secrets-service.test.ts` passed (`39/39`). - After Greptile follow-up commits: `pnpm --filter @paperclipai/server typecheck` passed. - Captured Storybook screenshots from `ui/storybook-static` for visual review. - Latest PR checks on `5ca3a5cf`: `policy`, serialized server suites 1/4-4/4, `Canary Dry Run`, `e2e`, `security/snyk`, and `Greptile Review` pass; aggregate `verify` is still registering the completed child checks. - Greptile review loop continued through the latest requested pass; all Greptile review threads are resolved and the latest `Greptile Review` check on `5ca3a5cf` passed with 0 comments added. ## Screenshots Before: the provider-vault and remote-import surfaces did not exist on `master`; these are after-state screenshots from the Storybook fixtures.    ## Risks - Migration risk: this adds new secret provider tables and extends existing secret rows. The migrations were checked for monotonic ordering and idempotent guards, but reviewers should still inspect upgrade behavior carefully. - Provider risk: AWS support uses direct SigV4 requests. Automated tests cover signing, request timeouts, vault-config selection, namespace guardrails, pending-version archival, sanitized provider errors, and service-level cleanup paths. A real-vault AWS smoke test remains deployment validation for an operator with AWS credentials rather than an unverified merge blocker in this local branch. - UI risk: the Secrets page and import dialog are large new surfaces; screenshots are included above for reviewer inspection. - Verification risk: the full local stable test command hit parallel-load timing failures, although the exact failed files passed when rerun directly. - Operational risk: remote import intentionally avoids plaintext reads; operators must understand that imported external references resolve at runtime and may fail if AWS permissions change. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex, GPT-5 coding agent with local shell/tool use in the Paperclip worktree. Exact context-window size was not exposed by the runtime. ## 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 - [ ] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] If this change affects the UI, I have included before/after screenshots - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> |
