Add secrets provider vaults and remote import#5429
Conversation
|
@greptile review |
Greptile SummaryThis PR adds a company-scoped secrets control plane to Paperclip, including provider vault configuration, an AWS Secrets Manager provider using custom SigV4 signing, remote-import (preview + commit) from AWS, binding/access-event infrastructure, and matching UI and CLI surfaces. It also addresses a broad set of previously-flagged correctness issues in the secrets service.
Confidence Score: 5/5Safe to merge after addressing the unbounded access-events query; the core create/rotate/delete flows are now retry-safe and the previously-flagged soft-delete and TOCTOU defects are fixed. The iteration of correctness fixes is thorough: reservation-pattern create, active-status guard on rotate, status-aware filtering in list and conflict maps, disabled-vault bypass in remove, and pathPrefix-scoped DELETE in syncEnvBindingsForTarget are all present and verified in the code. The remaining finding is a missing limit on the access-events query, which is a scalability concern rather than a data-correctness bug. server/src/services/secrets.ts — Important Files Changed
Prompt To Fix All With AIFix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
server/src/services/secrets.ts:1145-1150
**`listAccessEvents` returns unbounded results**
`listAccessEvents` issues a `SELECT` with no `.limit()`. For a secret resolved on every heartbeat or in high-frequency routines, this table can accumulate tens of thousands of rows. The `GET /secrets/:id/access-events` route returns all of them in a single response, which can cause large payloads, slow queries, and OOM pressure on the Node process. Consider adding a default limit (e.g., `.limit(500)`) and a cursor-based or offset pagination parameter.
Reviews (27): Last reviewed commit: "filter deleted secrets in remote import ..." | Re-trigger Greptile |
|
@greptile review |
|
@greptile review |
|
@greptile review |
|
@greptile review |
|
@greptile review |
a9f7dfd to
c59830d
Compare
|
@greptile review |
3 similar comments
|
@greptile review |
|
@greptile review |
|
@greptile review |
|
Update for PAP-9065: branch is on top of (rebase clean, no conflicts), all local changes are already committed, branch is in sync with remote (), and PR body already follows the template sections (Thinking Path, What Changed, Verification, Risks, Model Used, Checklist). is not included in PR diff. PR checks currently show all required CI jobs passing except Greptile Review is pending. Merge state is MERGEABLE (UNSTABLE due pending checks). |
|
PAP-9065 update
|
d7b27fe to
cdc7aa6
Compare
|
@greptile review |
8fca445 to
0ad3813
Compare
|
@greptile review |
0ad3813 to
671f238
Compare
|
@greptile review |
… master (#58) Adds a transitional warning before §Setup explaining that on paperclipai master (after the new Secrets Manager merge), plugin secret-ref UUIDs are temporarily disabled while a company-scoped plugin_config follow-up lands. Recommends pinning to the last paperclipai release before #5429 until upstream restores secret-ref resolution. Callout will be removed in a follow-up. Refs paperclipai/paperclip#5429 Refs paperclipai/paperclip:doc/plans/2026-04-26-plugin-secret-ref-company-scope.md Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… master (#25) Adds a transitional warning before §Setup explaining that on paperclipai master (after the new Secrets Manager merge), plugin secret-ref UUIDs are temporarily disabled while a company-scoped plugin_config follow-up lands. Recommends pinning to the last paperclipai release before #5429 until upstream restores secret-ref resolution. Callout will be removed in a follow-up. Refs paperclipai/paperclip#5429 Refs paperclipai/paperclip:doc/plans/2026-04-26-plugin-secret-ref-company-scope.md Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… master (#46) Adds a transitional warning before §Setup explaining that on paperclipai master (after the new Secrets Manager merge), plugin secret-ref UUIDs are temporarily disabled while a company-scoped plugin_config follow-up lands. Recommends pinning to the last paperclipai release before #5429 until upstream restores secret-ref resolution. Callout will be removed in a follow-up. Refs paperclipai/paperclip#5429 Refs paperclipai/paperclip:doc/plans/2026-04-26-plugin-secret-ref-company-scope.md Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Brings in upstream's recent commits onto the fork that already carried the five Tier-1 plans (Memory, Artifacts, Work Queues, Deep Planning, Org Learning): - f784d8d Retry canary registry verification (paperclipai#5579) - 06e6ee2 Add Daytona sandbox provider plugin (paperclipai#5580) - 778e775 Add secrets provider vaults and remote import (paperclipai#5429) - 433dfed Enable CI publish for plugin-daytona (paperclipai#5586) - e3af7aa Add shared sidebar section controls (paperclipai#5585) Conflicts resolved: * Migration numbering. Upstream's 0082_dry_vision and 0083_company_secret_provider_configs collided with the fork's 0082..0090 chain. Renumbered the fork's nine migrations to 0084..0092 (and matching meta/ snapshot files) so the journal lists upstream's secrets-vault migrations at 82-83 followed by the fork's distributed-workers, workspace-leases, memory, artifacts (+ legacy view), work queues, deep planning, routine.requires_plan, and org learning at 84-92. Verified e2e against a fresh embedded Postgres run: 93 migrations apply cleanly in order. * packages/shared/src/constants.ts. Upstream added SECRET_PROVIDER_CONFIG_STATUSES + SECRET_STATUSES + related constants and bumped STORAGE_PROVIDERS to ["local_disk", "s3"]; the fork had already added "gcs" to STORAGE_PROVIDERS in distributed-workers Plan 5. Kept all upstream secret constants AND restored "gcs" in the storage list. * ui/src/App.tsx. Upstream added eager `import { Secrets } from "./pages/Secrets"`; the fork had refactored every route to use lazyNamed() for vite chunk-splitting (~3.6 MB → per-route chunks). Kept the lazyNamed pattern and added Secrets via the same wrapper. * ui/src/components/SidebarAgents.tsx. Upstream added persisted agent sort modes via lib/agent-order; the fork had added useIsVisible from hooks/useIsVisible. Kept both imports. Verified: - pnpm install: lockfile already up to date. - pnpm -r exec tsc --noEmit: 0 errors. - /tmp/paperclip-e2e/run-e2e.sh: PASSED on a freshly-booted server with all 93 migrations applied: company → agent → issue → plan(2 phases + decision) → playbook + suggest → work-queue with Idempotency-Key dedupe → artifacts list. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The merge of upstream's paperclipai#5429 (provider vaults + AWS Secrets Manager) added a new 2,155-line Secrets.tsx page covering everything the fork's 528-line CompanySecrets page did, plus vault management and AWS import. Auto-merge took both sides' sidebar entry and route registration, so React Router picked the fork's older page first and upstream's AWS UI never rendered. - ui/src/App.tsx: drop CompanySecrets import + duplicate route at /company/settings/secrets - ui/src/components/CompanySettingsSidebar.tsx: drop duplicate Secrets nav item - ui/src/pages/CompanySecrets.tsx: delete (superseded) Server-side delete guard (services/secrets.ts: refuse delete when secret is referenced by agents or skills) is preserved and still enforced; upstream's page surfaces the API error message in place of the fork's inline reference list.
Now that upstream's paperclipai#5429 provides provider-based secrets management with formal bindings (companySecretBindings table populated at config-save time) plus the /secrets/:id/usage endpoint backed by listBindingReferences(), the fork's parallel usages() scan is redundant for agent/routine bindings. The fork's scan did cover one path upstream doesn't track: skill metadata sourceAuthSecretId references. Dropping this means accidental deletion of a skill's auth secret is no longer rejected — accepted as a chase-upstream tradeoff. - server/src/services/secrets.ts: drop usages(), SecretUsage* types, in-use guard in remove(), and companySkills/agentService imports - server/src/routes/secrets.ts: drop GET /secrets/:id/usages route - ui/src/api/secrets.ts: drop usages() client method Typechecks clean on server and ui.
## 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>
Upstream PR paperclipai#5429 (Add secrets provider vaults and remote import, 2026-05-09) replaced the working secret-ref resolver with an unconditional "fail closed" gate that throws: "Plugin secret references are disabled until company-scoped plugin config lands" The same PR also added an operator-level block in routes/plugins.ts that returns HTTP 422 on any plugin config save containing a secret-ref UUID. Together they break every plugin that resolves secrets through ctx.secrets.resolve() — paperclip-plugin-telegram fails worker activation ("Activation failed: Worker initialize failed... Plugin secret references are disabled..."), paperclip-plugin-slack and paperclip-plugin-discord would hit the same path. The cited "company-scoped plugin config" feature hasn't fully landed; the gate is a placeholder that ships in main and silently regresses every installed plugin. Restore the pre-paperclipai#5429 implementation of plugin-secrets-handler.ts (which looks up the secret by UUID, fetches the latest version material via @paperclipai/db, and delegates to the configured SecretProviderModule) and remove the matching 422 short-circuit in routes/plugins.ts. The operator-level audit log still records the refs. This is a temporary forward-port carry until upstream lands the new company-scoped path. When that ships, this patch should be reverted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three changes to keep CI green after merging upstream/master v2026.517.0: 1. company-documents-routes.test.ts — add issueRecoveryActionService to the ../services/index.js mock. Upstream PR paperclipai#5599 (source-scoped recovery actions) added the service; the route handler now imports it via the service barrel, and our mock didn't return it, so the tests crashed with "No 'issueRecoveryActionService' export is defined on the mock". 2. plugin-routes-authz.test.ts — skip the one test that asserts the upstream PR paperclipai#5429 "fail closed" gate (HTTP 422 on plugin config saves containing secret refs). Our companion patch in services/plugin-secrets-handler.ts restores the working secret-resolution path so installed plugins keep functioning. Revert this skip when upstream lands company-scoped plugin config. 3. plugin-secrets-handler.test.ts — skip the entire describe block testing the same gate, and drop the import of the now-removed PLUGIN_SECRET_REFS_DISABLED_MESSAGE constant. Same revert-with- upstream-feature plan as (2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The secrets-vault merge (paperclipai#5429) left a 422 guard that rejects any plugin config containing secret references. With the resolver re-enabled (dbb085b), that guard is the last blocker for paperclip-plugin-whatsapp, whose config stores 4 tokens as company-secret refs — the guard made the plugin impossible to (re)configure and broke inbound WhatsApp. Drop it.
> - 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. - 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. - `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. Before: the provider-vault and remote-import surfaces did not exist on `master`; these are after-state screenshots from the Storybook fixtures.    - 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`. - 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. - [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>
## 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 `5352e1d8` 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>
Upstream's "fail closed" gate in PR paperclipai#5429 (plugin secret references disabled until company-scoped plugin config lands) still hasn't been replaced as of v2026.609.0 — over a month. It bricks every plugin that resolves secrets through ctx.secrets.resolve (Telegram, Slack, etc.). This restores the pre-paperclipai#5429 working implementation: - Look up the secret by UUID from companySecrets - Fetch the latest version material from companySecretVersions - Delegate to the configured SecretProviderModule for resolution - Cache allowed-refs per plugin for 30s to avoid repeated config lookups Plus removes the matching operator-level 422 short-circuit in routes/plugins.ts that blocks saving plugin configs containing secret refs. The two test files that assert the gate are .skip-marked with comments pointing at this patch. Revert all four files when upstream lands company-scoped plugin config. This is the only remaining fork patch — the Documents view feature was superseded by upstream's Company Artifacts (PR paperclipai#7621, v2026.609.0) and PR paperclipai#4613 has been closed. The create-paperclip-plugin esbuild bundle guard was fixed upstream and is no longer needed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operator secrets and provider vaults let a company connect external secret stores without copying every value manually. > - The existing AWS Secrets Manager import dialog already supports importing remote secrets from any eligible vault. > - From the Provider vaults tab, operators can see a specific vault row, but there was no row-level import action that preserved that vault context. > - That meant importing from a specific vault required opening the generic import dialog and reselecting the intended provider config. > - This pull request adds a row-level refresh/import action that opens the import dialog with the selected vault preselected. > - The benefit is a tighter, less error-prone flow when a company has multiple AWS provider vaults configured. ## Linked Issues or Issue Description No public issue exists for this small UI follow-up. Problem description: - Users with multiple AWS Secrets Manager provider vaults need to refresh/import secrets from the vault they are currently looking at. - The existing import dialog defaulted to the configured default vault or first eligible vault, even when the operator initiated the flow from a specific provider-vault row. - This creates extra selection work and makes it easier to import from the wrong vault. Related public PRs: Refs #5429, #8586. ## What Changed - Added a Provider vaults row action for AWS Secrets Manager configs to refresh/import existing remote secrets. - Threaded an optional initial provider config id into `ImportFromVaultDialog` so the selected vault is preselected when eligible. - Reset the initial vault selection when the dialog closes or routes to vault management. - Added render coverage proving the row action opens the import dialog and previews against the selected vault id. ## Verification - `pnpm exec vitest run ui/src/pages/Secrets.render.test.tsx` passed: 1 file, 9 tests. - `pnpm --filter @paperclipai/ui typecheck` passed. ## Risks Low risk. This is UI-only behavior scoped to AWS Secrets Manager provider-vault rows and the existing import dialog. The main risk is that future provider types may need their own row-level import labels or eligibility rules instead of sharing this AWS-specific action. > 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 GPT-5 Codex via Paperclip/Codex local agent, with repository file access, shell command execution, GitHub CLI, and TypeScript/Vitest verification. Exact context-window metadata was not surfaced in this 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: Claude Opus 4.8 <noreply@anthropic.com>
## 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 `47b9bbec` 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 `073dc184`: `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 `073dc184` 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>
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operator secrets and provider vaults let a company connect external secret stores without copying every value manually. > - The existing AWS Secrets Manager import dialog already supports importing remote secrets from any eligible vault. > - From the Provider vaults tab, operators can see a specific vault row, but there was no row-level import action that preserved that vault context. > - That meant importing from a specific vault required opening the generic import dialog and reselecting the intended provider config. > - This pull request adds a row-level refresh/import action that opens the import dialog with the selected vault preselected. > - The benefit is a tighter, less error-prone flow when a company has multiple AWS provider vaults configured. ## Linked Issues or Issue Description No public issue exists for this small UI follow-up. Problem description: - Users with multiple AWS Secrets Manager provider vaults need to refresh/import secrets from the vault they are currently looking at. - The existing import dialog defaulted to the configured default vault or first eligible vault, even when the operator initiated the flow from a specific provider-vault row. - This creates extra selection work and makes it easier to import from the wrong vault. Related public PRs: Refs paperclipai#5429, paperclipai#8586. ## What Changed - Added a Provider vaults row action for AWS Secrets Manager configs to refresh/import existing remote secrets. - Threaded an optional initial provider config id into `ImportFromVaultDialog` so the selected vault is preselected when eligible. - Reset the initial vault selection when the dialog closes or routes to vault management. - Added render coverage proving the row action opens the import dialog and previews against the selected vault id. ## Verification - `pnpm exec vitest run ui/src/pages/Secrets.render.test.tsx` passed: 1 file, 9 tests. - `pnpm --filter @paperclipai/ui typecheck` passed. ## Risks Low risk. This is UI-only behavior scoped to AWS Secrets Manager provider-vault rows and the existing import dialog. The main risk is that future provider types may need their own row-level import labels or eligibility rules instead of sharing this AWS-specific action. > 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 GPT-5 Codex via Paperclip/Codex local agent, with repository file access, shell command execution, GitHub CLI, and TypeScript/Vitest verification. Exact context-window metadata was not surfaced in this 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: Claude Opus 4.8 <noreply@anthropic.com>
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Operator secrets and provider vaults let a company connect external secret stores without copying every value manually. > - The existing AWS Secrets Manager import dialog already supports importing remote secrets from any eligible vault. > - From the Provider vaults tab, operators can see a specific vault row, but there was no row-level import action that preserved that vault context. > - That meant importing from a specific vault required opening the generic import dialog and reselecting the intended provider config. > - This pull request adds a row-level refresh/import action that opens the import dialog with the selected vault preselected. > - The benefit is a tighter, less error-prone flow when a company has multiple AWS provider vaults configured. ## Linked Issues or Issue Description No public issue exists for this small UI follow-up. Problem description: - Users with multiple AWS Secrets Manager provider vaults need to refresh/import secrets from the vault they are currently looking at. - The existing import dialog defaulted to the configured default vault or first eligible vault, even when the operator initiated the flow from a specific provider-vault row. - This creates extra selection work and makes it easier to import from the wrong vault. Related public PRs: Refs paperclipai#5429, paperclipai#8586. ## What Changed - Added a Provider vaults row action for AWS Secrets Manager configs to refresh/import existing remote secrets. - Threaded an optional initial provider config id into `ImportFromVaultDialog` so the selected vault is preselected when eligible. - Reset the initial vault selection when the dialog closes or routes to vault management. - Added render coverage proving the row action opens the import dialog and previews against the selected vault id. ## Verification - `pnpm exec vitest run ui/src/pages/Secrets.render.test.tsx` passed: 1 file, 9 tests. - `pnpm --filter @paperclipai/ui typecheck` passed. ## Risks Low risk. This is UI-only behavior scoped to AWS Secrets Manager provider-vault rows and the existing import dialog. The main risk is that future provider types may need their own row-level import labels or eligibility rules instead of sharing this AWS-specific action. > 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 GPT-5 Codex via Paperclip/Codex local agent, with repository file access, shell command execution, GitHub CLI, and TypeScript/Vitest verification. Exact context-window metadata was not surfaced in this 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: Claude Opus 4.8 <noreply@anthropic.com>
Thinking Path
What Changed
public-gh/masterat0e1a5828and force-pushed with lease to keep this as the single PR for the branch.Verification
git fetch public-gh mastergit rebase public-gh/mastergit diff --name-only public-gh/master...HEAD | grep '^pnpm-lock\.yaml$' || trueconfirmedpnpm-lock.yamlis not in the PR diff.0081_optimal_dormammu; this PR adds0082_dry_visionand0083_company_secret_provider_configs.IF NOT EXISTS; foreign keys are guarded byDO $$ ... IF NOT EXISTS; column additions useADD COLUMN IF NOT EXISTS.pnpm -r typecheckpassed before the Greptile follow-up commits.pnpm test:runran 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, andenvironment-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.tspassed on targeted rerun (24/24).pnpm buildpassed before the Greptile follow-up commits. Vite reported existing chunk-size/dynamic-import warnings.pnpm --filter @paperclipai/server exec vitest run src/__tests__/secrets-service.test.tspassed (26/26).pnpm --filter @paperclipai/server exec vitest run src/__tests__/aws-secrets-manager-provider.test.ts src/__tests__/secrets-service.test.tspassed (39/39).pnpm --filter @paperclipai/server typecheckpassed.ui/storybook-staticfor visual review.5ca3a5cf:policy, serialized server suites 1/4-4/4,Canary Dry Run,e2e,security/snyk, andGreptile Reviewpass; aggregateverifyis still registering the completed child checks.Greptile Reviewcheck on5ca3a5cfpassed 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
Model Used
Checklist