fix(secrets): preserve binding egress posture across delete+reinsert syncs

[claudegoogl-sudo]

Follows CONTRIBUTING.md PR template — sections present: Thinking Path, Linked Issues or Issue Description, What Changed, Verification, Risks, Model Used, Checklist.

Thinking Path

• Paperclip is the open source app people use to manage AI agents for work
• Agents borrow secrets through company_secret_bindings rows. Each row carries an egress posture: an operator-set allowedEgress allowlist plus an egressAllowlistEnforced flag.
• Config saves reconcile bindings with a delete-then-reinsert. That re-creates rows with fresh ids. Posture is operator state keyed by the binding identity (companyId, targetType, targetId, configPath), not by the row id, so each sync must carry it over explicitly.
• Three of the four sync paths carried it over with hand-rolled copies. The fourth, replaceSecretRefsForInstanceTarget, re-inserted bare. Every instance-target config save silently reset operator posture: the allowlist went back to [] (deny-all while enforcing) and a log-only posture was re-armed. Security state the operator set was lost with no warning, on every save.
• This pull request unifies the preserve semantics into one shared helper and routes all four write paths through it, so the paths cannot drift again.
• The benefit: a config re-save never silently changes egress posture in either direction, new bindings stay born enforcing, and the invariant is documented.

Linked Issues or Issue Description

No public GitHub issue exists for this change. Problem description, bug-report shape:

Description

A binding's egress posture is operator state keyed by the binding identity. replaceSecretRefsForInstanceTarget deleted every non-env.% binding for an instance-scoped target and re-inserted the new rows without allowedEgress or egressAllowlistEnforced, so each instance-target config save reset allowedEgress to [] and egressAllowlistEnforced to true. The three sibling syncs (syncSecretRefsForTarget, syncEnvBindingsForTarget, syncPluginSecretBindings) preserved posture with hand-rolled existing?.x ?? default copies; the drift between the copies is the defect.

Steps to reproduce

1. Seed a binding at (company, environment, env-1, apiKey) with allowedEgress = ['https://example.com'].
2. Call replaceSecretRefsForInstanceTarget({ targetType: 'environment', targetId: 'env-1' }, [{ secretId, configPath: 'apiKey' }]).
3. Before this change, the re-inserted row has allowedEgress = []. After, it is unchanged.

Expected behavior

A config re-save preserves operator-set egress posture in both directions. A new binding at a path with no prior row is born enforcing with an empty allowlist (the column defaults).

Related merged work: #176 (security-posture column registry sweep) and #205 (operator-reachable egress posture panel) — this change is the write-path preservation rule those surfaces report on.

What Changed

• Added the shared preserve semantics to server/src/services/egress-posture.ts: preservedEgressPosture (carry the prior row's posture; fall back to the born-enforcing column defaults when there is no prior row), egressPostureCarryKey, and egressPostureCarryIndex.
• syncSecretRefsForTarget and syncEnvBindingsForTarget now build their carry index and resolve re-insert posture through the shared helper. Behavior is unchanged; the hand-rolled copies are gone.
• replaceSecretRefsForInstanceTarget.writeBindings now captures the posture of the rows it is about to delete (inside the same executor/transaction) and carries it onto the re-inserted rows. This is the fix: instance-target config saves no longer wipe operator allowlists or flip enforcement.
• syncPluginSecretBindings captures plugin-binding posture before its revoke deletes, so a repoint (delete+reinsert of the same config path) carries the old posture. The upsert's conflict branch still only updates secretId, so live rows stay untouched.
• Documented the invariants in doc/SECURITY-POSTURE-COLUMN-SWEEP.md: posture is preserved across delete+reinsert; new bindings are born enforcing; a legacy log-only row stays log-only until an operator flips it.
• Tests: server/src/__tests__/egress-posture-carry.test.ts (helper unit tests) and server/src/__tests__/secret-binding-egress-posture-preserve.test.ts (embedded-postgres tests, one per observable behavior).

Verification

Red-then-green, measured against base 14a4d4b59 (current master) and this branch:

• cd server && pnpm exec vitest run src/__tests__/secret-binding-egress-posture-preserve.test.ts
  • On base 14a4d4b59: Tests 4 failed | 3 passed (7) — the failures are exactly the posture wipe:
    • AssertionError: expected [] to deeply equal [ 'https://example.com' ] (replace path drops the operator allowlist)
    • AssertionError: expected true to be false (replace path re-arms a log-only posture)
    • expected [] to deeply equal [ 'https://a.example' ] (per-company posture on a shared instance target)
    • expected [] to deeply equal [ 'https://example.com' ] (plugin repoint reset)
  • On this branch: Test Files 1 passed (1), Tests 7 passed (7).
• cd server && pnpm exec vitest run src/__tests__/egress-posture-carry.test.ts — Tests 3 passed (3) (helper unit tests).
• Regression sweep of the neighbouring binding suites: pnpm exec vitest run src/__tests__/secret-binding-resilience.test.ts src/__tests__/plugin-secret-bindings-sync.test.ts src/__tests__/egress-posture-deny-all-sweep.test.ts — Tests 30 passed (30).
• Typecheck: pnpm exec tsc --noEmit in server/ — no errors in the touched files. The pre-existing module-resolution errors from unbuilt workspace deps are identical to base.
• The instance-target delete is not company-scoped (an environment is shared across companies), so the carry index is keyed by (companyId, configPath); the per-company test covers that case directly.

Risks

• Behavior change on replaceSecretRefsForInstanceTarget: instance-target config saves now preserve operator-set posture instead of resetting it. That is the fix. Any caller that relied on re-save resetting posture to deny-all must use the operator binding API, which is the documented surface.
• Plugin config repoints now carry the old path's posture onto the re-bound secret instead of resetting it. Same rationale: posture is operator state keyed by the path, not by the secret id.
• No migration and no schema change. Historical log-only rows stay log-only until an operator flips them; reconciling them is a separate, operator-gated change and is out of scope here.
• Low interaction risk with the deny-all sweep: preservation writes the same posture the row already had, so the sweep's dedup clock is untouched.

Model Used

Claude Opus 4.6 (claude-opus-4-7) via Claude Code (Paperclip agent runtime), extended thinking + tool use: local file edits, bash execution, embedded-postgres test runs, and the red-then-green A/B against the base commit.

Checklist

• I have included a thinking path that traces from project context to this change
• I have specified the model used (with version and capability details)
• I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work
• I have searched GitHub for duplicate or related PRs and linked them above
• I have either (a) linked existing issues with Fixes: # / Closes # / Refs # OR (b) described the issue in-PR following the relevant issue template
• 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
• I have run tests locally and they pass
• I have added or updated tests where applicable
• I have updated relevant documentation to reflect my changes
• I have considered and documented any risks above
• All Paperclip CI gates are green
• [N/A] Greptile is 5/5 with no open P2s, recommendations, or follow-ups — Greptile is not active on this fork; the upstream-only gate is documented in CONTRIBUTING.md under "Not applicable here".
• I will address all Greptile and reviewer comments before requesting merge