[feat] Managed vault secrets: platform-owned connections users cannot edit or delete

[mmabrouk]

Context

Agenta provisions some Vault connections and owns their lifecycle, but the earlier model exposed a free-form managed_by component string to clients and used a universal allow_managed bypass. That mixed internal identity, user mutation policy, value visibility, and frontend presentation.

Changes

This PR introduces a typed managed-secret model without a database migration.

Internal storage:

{"management": {"manager": "starter-credits-bridge", "policy": "manager_only"}}

Public response:

{"management": {"policy": "manager_only"}}

The manager identity stays internal. Public create and update DTOs cannot claim or clear management. Trusted components create a managed row through create_managed_secret with a typed SecretManager and SecretManagementPolicy.

General update and delete operations have no boolean bypass. They load and lock the current row, invoke a service-owned resolver or authorizer, and reject a manager-only row with HTTP 409 in the same transaction. The DAO owns transaction mechanics but does not interpret policy.

Management and write_only remain independent. Existing rows have no management object and stay unmanaged. The object lives in the existing encrypted JSON payload, so this needs no schema migration.

Tests / notes

• The combined write-only and managed-secret suite passed 74 tests after the final webhook cleanup.
• Final combined API verification passed 229 tests after removing four unsupported write-only webhook compatibility cases.
• Ruff formatting and checks passed.
• No migration, feature flag, ownership-clear path, or speculative owner bypass is included.

What to QA

• List a managed row. The response must expose management.policy but never the internal manager.
• Try public update and delete. Both must return HTTP 409 and leave the locked row unchanged.
• Create, update, and delete an ordinary row. Existing behavior must remain unchanged.
• Confirm management and value visibility vary independently. Neither policy may imply the other.

Depends on #6164 through the write-only-secrets-api base.

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
agenta-documentation	Ready	Preview	Aug 23, 2026 2:25pm

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

🗂️ Base branches to auto review (1)

• release/.*

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 835e0228-a605-4590-af79-108915c7192e

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[mmabrouk]

Reviewer guide for this PR. It ships inert - nothing is managed until a component sets a marker, no row carries one today, and there is no migration - so the review is really about whether the guard is in the right place and fails in the right direction. The inline notes cover the allowlist of what a user may still change, the pin-and-refuse mechanics, the default-denies parameter, and the one place where an omitted field could silently un-manage a row.

[github-actions]

Railway Preview Environment

Status	Destroyed (PR closed)

Updated at 2026-08-23T20:46:24.026Z

[mmabrouk]

Review round 3 (final heads)

Head 6c70e7b3c9, base write-only-secrets-api at d1e4f8832f.

The lane content is unchanged since round 2 (362860c6d1). The only difference is a line offset in dao.py from the rebase onto the new base. So this round is a re-verification plus a look at what the new base changes underneath.

Tests run read-only: pytest oss/tests/pytest/unit/secrets/ oss/tests/pytest/unit/vault/ -q → 119 passed.

Verified addressed

The seven reviewer-guide comments on this PR all describe an earlier design: a free-form managed_by string, an allowlist of still-editable fields, pin-and-refuse with rewrite-back, and an allow_managed parameter on update and delete. None of that is in the code now. GitHub marks all seven threads outdated, correctly.

What replaced it is simpler and I prefer it. A managed row is read-only, full stop.

• management.policy is the only public half. PublicSecretManagementDTO carries policy and nothing else (api/oss/src/core/secrets/managed.py:20-23), and project_secret_response builds it by hand rather than dumping the internal model (api/oss/src/core/secrets/redaction.py:97-99). The manager identity cannot reach a caller.
• Internal ownership is typed, not free-form. SecretManager is an enum (managed.py:5-7), not a string.
• No managed_by marker remains anywhere in the API or SDK. The only hits are two negative assertions in the tests.
• Update and delete both refuse with 409, and both refuse under the DAO row lock. _resolve_update is passed into SecretsDAO.update, which selects .with_for_update() before calling it (api/oss/src/dbs/postgres/secrets/dao.py:124-160). _authorize_delete is passed into SecretsDAO.delete, which now also selects .with_for_update() before authorizing (dao.py:181-198). Adding the lock to the delete path is the right call.
• A client cannot claim the marker. CreateSecretDTO and UpdateSecretDTO now set extra="forbid", so a body carrying management is rejected with 422 before any handler runs. The parametrized test covers both management and the old managed_by spelling (test_managed_routes.py:98).
• No migration needed. The marker rides inside the encrypted data JSON beside write_only, and map_secrets_dto_to_dbe_update reads the stored value back before the wholesale data rewrite so an update that omits it cannot un-manage the row (mappings.py:83-99).

Nit

Forward compatibility on read: an unknown manager makes the row unreadable. mappings.py:107 pops the stored management dict and hands it to SecretResponseDTO, where SecretManagementDTO has extra="forbid" and a closed manager enum. If a later version writes a second manager value, or adds a field, every build that predates it raises a ValidationError on read of that row, not only on edit. That includes a rollback and a mixed-version deploy. Today there is exactly one manager and no such row exists, so nothing is broken. It is worth deciding now whether reads should tolerate an unknown manager, because the cost of getting it wrong is a row nobody can read rather than a row nobody can edit.

Dead branch in _create_secret. api/oss/src/core/secrets/services.py:327-339 branches on management is None and calls secrets_dao.create twice with the only difference being whether the management kwarg is passed. The DAO parameter already defaults to None (dao.py:54), so both arms do the same thing. One call passing management=management reads better.

Not a defect, but worth stating in the description

Managed rows are not hidden on the wire. There is no query flag and no server-side filter. GET /secrets/ returns managed rows like any other, with management.policy set. Hiding them from Settings and keeping them in model selection is entirely a client decision, made in AIProvidersPage.tsx and connectionPicker.ts. Those files are not in this PR. That split is a reasonable design, since the runtime and the picker want the row and only the edit surfaces do not. It does mean the Settings page shows a row users cannot edit until the frontend lane lands. Worth naming in the PR body so the ordering is explicit.

Verdict

Safe to merge in order. The guard is in the right place, it fails closed, and it is enforced under the row lock on both mutating paths.

[mmabrouk]

lgtm