fix: support secret: prefixed UUID refs in ctx.secrets.resolve()

[r3ptar]

Thinking Path

• Paperclip orchestrates AI agents for zero-human companies

• Plugins extend Paperclip with integrations like Microsoft 365, and these plugins often need to store sensitive credentials like OAuth tokens and API keys

• The platform provides a secure secrets API so plugins don't have to store raw values in plaintext state

• When a plugin stores a secret through the platform, it gets back a UUID reference (sometimes prefixed as secret:)

• But ctx.secrets.resolve() only accepted bare UUIDs, and a config-only scope check rejected any UUID not already present in the plugin's instance config

• This meant plugins that created secrets at runtime could never resolve them, forcing authors to store raw secret values in ctx.state as plaintext in the database — a security risk

• This PR fixes ctx.secrets.resolve() to accept both bare and secret:-prefixed UUIDs, and removes the overly restrictive scope check so runtime-created secrets can be resolved

• The benefit is that plugin authors can now use the platform's secret storage as intended, keeping credentials encrypted rather than stored as plaintext in plugin state

  What I did

  Added parseSecretRef() which strips the secret: prefix, validates UUID format, enforces a 256-char length limit, and rejects empty strings. Removed the config-based scope check that was blocking runtime-created secrets — the UUID itself is a 122-bit unguessable capability token, and the existing 30/min rate limiter prevents enumeration. Cleaned up the dead imports and unused cache code that resulted from the scope check removal. Updated JSDoc throughout to document the new format.

  Why it matters

  Plugin authors were forced to store raw secret values in ctx.state (plaintext in the database) because ctx.secrets.resolve() couldn't handle the UUID refs the platform returned. This fix closes that gap so the platform's encrypted secret storage works end-to-end.

  How to verify

  • All 87 server test files pass (447 tests, 0 failures)
  • TypeScript compiles cleanly with tsc --noEmit
  • 19 new unit tests cover parseSecretRef (bare UUID, prefixed UUID, uppercase, empty string, prefix-only, excessive length, double prefix, non-UUID rejection) and extractSecretRefsFromConfig (null config, schema paths, nested paths, fallback mode, arrays)

  Risks

  The scope check removal means any plugin with secrets.read-ref capability can resolve any secret by UUID, not just ones in its config. This is mitigated by UUID entropy making guessing infeasible, the 30/min rate limiter, and capability gating. The previous scope check provided defense-in-depth but blocked the primary use case.

[greptile-apps]

Greptile Summary

This PR extends the plugin credential-resolution handler to accept prefixed UUID refs (e.g. "secret:uuid") alongside bare UUIDs. The new parseSecretRef() helper is clean and well-tested, and the description follows the CONTRIBUTING.md thinking-path format. Two issues need addressing before merge:

• Cross-company isolation is no longer enforced at the DB layer. The previous scope check implicitly restricted a plugin to refs present in its own config (which are company-scoped). The replacement query now filters only by id with no companyId constraint, violating the company-scoping invariant from AGENTS.md §5 rule 1. If host-client-factory already enforces this boundary, that should be documented; otherwise a companyId filter should be added to the lookup query.
• The in-memory rate limiter is process-local. The PR description cites 30 req/min rate limiting as the primary enumeration defence. In multi-instance deployments the limit multiplies by instance count and resets on restart, weakening the stated mitigation.

Unit-test coverage for parseSecretRef and extractSecretRefsFromConfig is thorough (19 tests). Rate-limit enforcement and cross-company isolation guarantees are not yet covered by tests.

Confidence Score: 2/5

• Not safe to merge without resolving the cross-company isolation gap introduced by removing the scope check.
• The core parsing fix is correct and well-tested, but the removal of the scope check removes implicit company-boundary enforcement without an equivalent replacement at the DB query level. AGENTS.md explicitly requires company scoping on every domain entity, and the in-memory rate limiter that is cited as the primary remaining defence is not effective in horizontally-scaled deployments.
• server/src/services/plugin-secrets-handler.ts — specifically the DB query in resolve() (missing company filter) and the createRateLimiter implementation (in-memory only).

Important Files Changed

Filename	Overview
server/src/tests/plugin-secrets-handler.test.ts	New test file with 19 unit tests covering parseSecretRef and extractSecretRefsFromConfig. Coverage of the new parsing logic is thorough (bare UUID, prefixed, uppercase, empty, double-prefix, excessive length, non-UUID). Missing tests for rate-limiting behaviour and cross-company isolation.
server/src/services/plugin-secrets-handler.ts	Core handler refactored to support prefixed UUID refs via new parseSecretRef(). The config-based scope check that provided implicit company-boundary enforcement was removed; the replacement (UUID entropy + in-memory rate limiting) is insufficient — the rate limiter is process-local and resets on restart, and no company filter is applied to the DB lookup query.

Comments Outside Diff (2)

1. server/src/services/plugin-secrets-handler.ts, line 270-284 (link)

   In-memory rate limiter not effective in multi-instance deployments

   The createRateLimiter implementation is process-local. Each server instance keeps its own Map<string, number[]>, and the counters are also reset on every process restart (e.g. deploys, crashes, rolling restarts).

   In a horizontally-scaled deployment with N instances, a plugin can make up to 30 × N resolution attempts per minute across instances while staying under the per-instance limit on each one. The PR description positions the 30/min rate limit as a primary defence against UUID enumeration now that the scope check is removed; if that defence is trivially bypassed in production, the security argument weakens significantly.

   Options to consider:

   • Use a shared counter (Redis INCR/EXPIRE, or a DB-backed counter) keyed on pluginId.
   • Document that rate limiting is best-effort and call out that defence-in-depth (e.g. monitoring, alerting on anomalous resolution volume) is expected to complement it.
   • Restore the company-scoped query filter (see other comment) as an independent, distributed-safe layer.
   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: server/src/services/plugin-secrets-handler.ts
   Line: 270-284
   
   Comment:
   **In-memory rate limiter not effective in multi-instance deployments**
   
   The `createRateLimiter` implementation is process-local. Each server instance keeps its own `Map<string, number[]>`, and the counters are also reset on every process restart (e.g. deploys, crashes, rolling restarts).
   
   In a horizontally-scaled deployment with `N` instances, a plugin can make up to `30 × N` resolution attempts per minute across instances while staying under the per-instance limit on each one. The PR description positions the 30/min rate limit as a primary defence against UUID enumeration now that the scope check is removed; if that defence is trivially bypassed in production, the security argument weakens significantly.
   
   Options to consider:
   - Use a shared counter (Redis `INCR`/`EXPIRE`, or a DB-backed counter) keyed on `pluginId`.
   - Document that rate limiting is best-effort and call out that defence-in-depth (e.g. monitoring, alerting on anomalous resolution volume) is expected to complement it.
   - Restore the company-scoped query filter (see other comment) as an independent, distributed-safe layer.
   
   How can I resolve this? If you propose a fix, please make it concise.

2. server/src/services/plugin-secrets-handler.ts, line 81-83 (link)

   Misleading "v4" comment on UUID regex

   The comment says "UUID v4 regex" but the pattern accepts any RFC-4122 UUID variant (v1, v3, v4, v5, v7, etc.) because it places no constraint on the version nibble (4th group, first hex digit) or the variant bits (5th group, first two bits).

   This is fine for the intended purpose — matching the company_secrets primary key regardless of how it was generated — but the comment should be updated to avoid confusion:

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: server/src/services/plugin-secrets-handler.ts
   Line: 81-83
   
   Comment:
   **Misleading "v4" comment on UUID regex**
   
   The comment says "UUID v4 regex" but the pattern accepts any RFC-4122 UUID variant (v1, v3, v4, v5, v7, etc.) because it places no constraint on the version nibble (4th group, first hex digit) or the variant bits (5th group, first two bits).
   
   This is fine for the intended purpose — matching the `company_secrets` primary key regardless of how it was generated — but the comment should be updated to avoid confusion:
   
   
   
   How can I resolve this? If you propose a fix, please make it concise.

Prompt To Fix All With AI

This is a comment left during a code review.
Path: server/src/services/plugin-secrets-handler.ts
Line: 326-330

Comment:
**Company isolation not enforced in secret lookup**

The query only filters by `companySecrets.id` with no company boundary check. AGENTS.md §5 rule #1 states: *"Every domain entity should be scoped to a company and company boundaries must be enforced in routes/services."*

Before this PR, the scope check (`cachedAllowedRefs`) provided implicit company isolation — a plugin's config would only reference secrets belonging to its own company, so a plugin from company A could never resolve a secret owned by company B. That protection is now gone.

With this change, any plugin holding the `secrets.read-ref` capability can resolve a secret from **any** company if it obtains the UUID (e.g. from a log, an error message, or a cross-tenant API leak). The UUID entropy argument is sound within a single company's secret namespace, but it does not prevent cross-company lateral movement once a UUID is known.

To restore the invariant, `companyId` should be threaded through `PluginSecretsHandlerOptions` and added to the WHERE clause:

```
// In PluginSecretsHandlerOptions
companyId: string;

// In the resolve query
.where(
  and(
    eq(companySecrets.id, secretId),
    eq(companySecrets.companyId, companyId),   // enforce company boundary
  )
)
```

If `host-client-factory` already prevents cross-company invocations at the capability layer, please add a comment here explaining why the DB-level check is safe to omit — otherwise this silently relies on external enforcement that isn't visible in this module.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/services/plugin-secrets-handler.ts
Line: 270-284

Comment:
**In-memory rate limiter not effective in multi-instance deployments**

The `createRateLimiter` implementation is process-local. Each server instance keeps its own `Map<string, number[]>`, and the counters are also reset on every process restart (e.g. deploys, crashes, rolling restarts).

In a horizontally-scaled deployment with `N` instances, a plugin can make up to `30 × N` resolution attempts per minute across instances while staying under the per-instance limit on each one. The PR description positions the 30/min rate limit as a primary defence against UUID enumeration now that the scope check is removed; if that defence is trivially bypassed in production, the security argument weakens significantly.

Options to consider:
- Use a shared counter (Redis `INCR`/`EXPIRE`, or a DB-backed counter) keyed on `pluginId`.
- Document that rate limiting is best-effort and call out that defence-in-depth (e.g. monitoring, alerting on anomalous resolution volume) is expected to complement it.
- Restore the company-scoped query filter (see other comment) as an independent, distributed-safe layer.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: server/src/services/plugin-secrets-handler.ts
Line: 81-83

Comment:
**Misleading "v4" comment on UUID regex**

The comment says "UUID v4 regex" but the pattern accepts any RFC-4122 UUID variant (v1, v3, v4, v5, v7, etc.) because it places no constraint on the version nibble (4th group, first hex digit) or the variant bits (5th group, first two bits).

This is fine for the intended purpose — matching the `company_secrets` primary key regardless of how it was generated — but the comment should be updated to avoid confusion:

```suggestion
/** Regex for validating a RFC-4122 UUID (any version). */
const UUID_RE =
  /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i;
```

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix: support secret: prefixed UUID refs ..." | Re-trigger Greptile}