[Asymtode712]

Summary

Steps 2 and 3 of #1273: managed mode is now enforced by the backend and reflected in the Web UI. Step 1 (policy type, --connection-mode, managed marker) landed in #1278.

Under --connection-mode managed-only, browser users can open connections and query within the database account's privileges. They cannot create, change, delete, duplicate, import, export or test connections, cannot redirect a connection, and cannot read stored credentials. Standalone Web and desktop are unchanged.

Every authenticated user of an instance shares all of its managed connections. There are no per-user grants; use a separate instance per group of users. Details in docs/web/WEB_MANAGED_MODE.md.

Backend (step 2)

• Denials: a policy check runs in RpcDispatcher::dispatch before any payload is decoded. Denied commands return 403 with managed_connection_denied and details.action.
  • Covered: save, update, delete, duplicate, test and list databases (caller-supplied params), connection import, exports, backups, tunnel profile changes, and appearance, groups and tags (gated by --managed-allow-branding / --managed-allow-organization).
  • Destination overrides (driver, host, port, database, tunnel, proxy, plugin settings) can only arrive through the denied commands.
  • The icon and connection-import upload endpoints are gated too and return the same JSON error envelope.
• Guard test: the command classification is an exhaustive match, so a new command without a policy entry does not compile. A test also reads the parse tables from the source and fails if a connection, file or tunnel command is missing from the matrix.
• Forged marker: an import (native or from another app) can no longer set managed on a new connection or change it on a replaced one. This closes the gap noted in feat(web): managed-mode connection policy foundation (#1273) #1278. Browser save and update payloads reject a managed field.
• Credentials: connection responses also clear SOCKS5 and proxy credentials. Error messages and pushed events are scrubbed of stored credentials (DB, SSH, URI, plugin, proxy). The event redactor refreshes every 30 seconds.
• Startup: managed-only refuses to start without --auth or together with --allow-high-risk, since local-administrator authority would defeat the model.

Frontend (step 3)

• Session negotiation reports a connections block (managed plus one flag per action). Older servers omit it and are treated as unrestricted.
• The Connections page hides add, import, edit, duplicate, delete, export, bulk actions, group tools and drag-to-group when unavailable, and shows a notice that the connections are managed. Group collapse stays local when groups are locked.
• The SSH and Backup settings tabs are hidden in managed mode.
• The UI maps the error by its code, not by server text. Strings are translated in all 13 locales.

Tests

• Rust: a policy matrix and guard test, dispatcher-level bypass tests, and end-to-end tests through the real HTTP server with distinctive secrets in file, keychain and plugin storage. They cover credential exposure, error and event scrubbing, import marker provenance, upload gating and startup validation.
• Web UI: session and transport validation, the denial code on RPC and uploads, error mapping, ActionButtons, the Connections page, Settings tabs, the provider, and locale parity.
• Checked: cargo test --lib, cargo check --no-default-features, pnpm lint, typecheck, build, and the web:* checks.
• Known: two theme tests fail locally on macOS because of the /var symlink, and are unrelated to this change.

Limitations

• Scrubbing replaces exact values, so values under four characters are not scrubbed and query results are not scrubbed. A rotated credential can appear in an event for up to 30 seconds.
• Connections are prepared before the instance starts. A provisioning API ([Feat]: Web API for provisioning and maintaining managed connections #1274) is the only planned way to change them while it runs.
• Out of scope: per-user grants, multi-tenant isolation, personal groups and tags, mixed mode, network restrictions and database privileges.

---

^{Open in [code]smith · Need help on this PR? Tag @codesmith-bot with what you need.}

[kilo-code-bot]

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The only change since the last review is formatting in DatabaseProvider.test.tsx. Long open_saved_connection mock lines are wrapped, and behavior is unchanged. I did not run any code or tests.

Files Reviewed (1 file)

• packages/web-ui/tests/contexts/DatabaseProvider.test.tsx

Previous Review Summaries (3 snapshots, latest commit 98be63f)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 98be63f)

Status: No Issues Found | Recommendation: Merge

I reviewed the incremental changes since 2f3e550. The main one is the new open_saved_connection command, which takes only a saved connection id.

• Saved-access check: the RPC reads the id from connectionId, so authorize_saved_access checks the saved connection's driver against the policy.
• Managed policy: it is marked Unrestricted. That is acceptable because it cannot probe a new target.
• Redaction: whitespace-only passwords are now redacted, and server startup applies the same rule when it refuses short credentials.

I did not run any code or tests.

Files Reviewed (17 files)

• src-tauri/src/application/connections.rs
• src-tauri/src/application/secret_redaction.rs
• src-tauri/src/transport/desktop/commands.rs
• src-tauri/src/transport/desktop/mod.rs
• src-tauri/src/transport/web/rpc.rs
• src-tauri/src/transport/web/rpc_managed_policy.rs
• src-tauri/src/transport/web/server.rs
• Test files and the frontend files were skimmed only.

Previous review (commit 2f3e550)

Status: No Issues Found | Recommendation: Merge

I reviewed the incremental changes since 8ebb470, which touch about 106 files. Most of that diff looks like newly merged table-transfer and DML-preview work. I read the web transport changes in detail:

• New RPC commands: the table-transfer and DML-preview commands are decoded with deny_unknown_fields.
• Connection policy: authorize_saved_access checks both the source and target connection against the driver policy for table transfers.
• Server startup: managed-only mode now refuses to start if any stored credential is too short to redact.

I found no new issues. I did not run any code or tests.

Files Reviewed

• src-tauri/src/transport/web/rpc.rs
• src-tauri/src/transport/web/rpc_connection_policy.rs
• src-tauri/src/transport/web/server.rs
• The remaining incremental files were skimmed only.

Previous review (commit 8ebb470)

Status: No Issues Found | Recommendation: Merge

This is a large PR (about 60 files). I read the Rust backend diff in detail: the managed policy dispatch, upload gating, startup validation, error scrubbing and the policy matrix tests. I only skimmed the Web UI and locale changes. I did not run any code or tests, so the PR's own test claims are unverified.

Observation, not a blocking finding: in managed-only mode every application error triggers a connection_secret_values reload before it is redacted (rpc.rs, scrub_error). This could add load if errors are frequent, because the reload may touch the keychain and plugin storage.

Files Reviewed (60 files)

• src-tauri/src/transport/web/rpc.rs
• src-tauri/src/transport/web/rpc_managed_policy.rs
• src-tauri/src/transport/web/rpc_managed_policy_tests.rs
• src-tauri/src/transport/web/rpc_managed_tests.rs
• src-tauri/src/transport/web/server.rs
• src-tauri/src/transport/web/managed_mode_tests.rs
• src-tauri/src/transport/web/mod.rs
• src-tauri/src/transport/web/rpc_tests.rs
• src-tauri/src/transport/web/tests.rs
• the remaining Rust backend files (application, cli, connection_import, lib, events, contract)
• the Web UI files, tests, locales and docs (skimmed)

---

_{Reviewed by claude-sonnet-5.5 · Input: 254 · Output: 605 · Cached: 142.7K}

_{Review guidance: REVIEW.md from base branch main}

[debba]

Thanks for putting the policy checks before payload decoding and for covering direct RPC calls, import markers and the upload endpoints. The exhaustive connection-command classification makes this much easier to review.

I need one credential-exposure case fixed before merging. SecretRedactor::replace drops stored secrets shorter than four bytes. I ran the actual helper on this branch with the stored value abc: redacting driver rejected password abc returned that same text, including the password. The existing ignores_values_too_short_to_scrub_safely test also explicitly preserves it. Both scrub_error and the event bus rely on this helper, so they have no protection for those credentials.

I saw that this is documented, but asking operators to use longer passwords does not enforce the managed-mode promise that browser users cannot read stored credentials. Please handle these values safely too, or reject a managed configuration containing unsupported credentials before it starts. Add error and event regressions for a short stored password so this boundary stays enforced. This is a helper-level reproduction; I am not claiming to have reproduced a leak through a real database driver.

[Asymtode712]

@debba thanks, you were right. A short stored password was skipped entirely, and the old test preserved that.

Fixed in the latest commit:

• The redactor never skips a stored secret now. A secret of 4+ characters is replaced in place. A shorter one replaces the whole string that contains it, so error messages, event payloads and JSON keys can't expose it. Your abc case now returns [redacted].
• managed-only refuses to start if any stored credential is shorter than 4 characters. The message gives only the count, not the values.
• If a short credential appears after startup (for example a rotation), the same whole-string redaction covers it.

Regression tests cover 1, 2 and 3 character passwords for errors, events and startup. They also cover the 4-character boundary, and standalone mode is unchanged. Docs and the --connection-mode help are updated.

[debba]

Thanks for following up on short passwords. I tried 2f3e550 against a real PostgreSQL fixture. The abc startup check now works, but I still hit two blockers.

1. Connect cannot open a managed connection. Start Web with password authentication and --connection-mode managed-only, sign in, and click Connect on an administrator-saved PostgreSQL connection. I expected the editor to open. Instead, the page stays on Connections with “Failed to connect”, and POST /api/v1/rpc/test_connection returns HTTP 403 with managed_connection_denied. DatabaseProvider.connect() still calls that forbidden RPC before registering the saved connection. The same connection opens and runs SELECT COUNT(*) FROM orders in standalone mode, and a direct get_tables call succeeds in managed mode, so the database is reachable. Please make the managed Connect path validate/open the saved connection by ID through an allowed backend operation, while keeping caller-supplied connection tests blocked. An integration test using the real connect flow would catch this.

2. Nonempty whitespace passwords still bypass the credential checks. I set the PostgreSQL password to exactly three ASCII spaces and verified a TCP login. With that same stored password, the managed server starts and get_tables succeeds, although the new startup rule says credentials shorter than four characters are rejected. Both the startup filter and SecretRedactor::replace() skip trim().is_empty() values. In a separate reproduction using the current redactor, SecretRedactor::new([" ".to_owned()]).redact("driver rejected password \" \"") returns the original text; four spaces are also left untouched. That last result is from the actual helper, not a captured PostgreSQL driver error. Please handle nonempty whitespace credentials in both checks and cover them in the startup, RPC-error and event tests.

I could not complete the managed query flow through the UI because of the first blocker.

[Asymtode712]

@debba Thanks for testing 2f3e550 against a real PostgreSQL fixture. Both blockers are fixed in the follow-up commit.

1. Connect in managed mode. DatabaseProvider.connect() and useOpenConnectionInNewWindow now call a new open_saved_connection RPC. It takes only a connection ID, loads the saved parameters on the backend and runs the same checks test_connection runs. Managed-only mode allows it because it can't target another host. test_connection and list_databases still return managed_connection_denied. File-backed drivers still need local administrator authorization.

2. Whitespace passwords. The startup check and SecretRedactor::replace() now skip only empty strings. A password of 1–3 whitespace characters is rejected at startup and blanks the whole string in errors and events. A whitespace password of 4 or more characters is replaced in place.

I added tests for both. They cover the managed open_saved_connection vs test_connection split over real HTTP, whitespace passwords at startup, in RPC errors, in events and in the redactor, and the frontend connect path.