feat: pin a default connection; fix Editor misdiagnosing malformed SQL as a permissions error

[venkateshsakamuri-lab]

Two independent changes, one commit each.

---

1. feat: pin a connection as your per-user default

Requested feature. A pin toggle in Manage Connections and in the sidebar connection switcher; a pinned connection is the one DeepSQL opens on every load.

The pin is per user, not a flag on the connection. A connection can be shared through connection_access_grant, so a column on database_connection would let one person's choice decide what everyone else opens on — and shared connections are canManageConfig=false for their recipients, so exactly the people who most want a default could not set one. New connection_pin table, one row per user, unique constraint on username; pinning a second connection moves the row rather than adding one.

PUT|DELETE /connections/{id}/pin are gated on assertCanUseConnection, not assertCanManageConnectionConfig — choosing where you land is a preference, not a change to the connection. GET /connections carries pinned per caller, so no surface needs a second request; deleteConnection clears every pin on the connection alongside its grants.

The pin has to beat an already-selected connection, not just an empty one. useDashboardStore persists connectionId, so after a reload something is always selected and the existing auto-select never ran. useConnectionManager now applies the pin once per page load, tracked at module scope rather than in a ref — the hook is called from a dozen sections, and a per-instance guard would let a later-mounted section yank the user back to the pin after they deliberately switched. Switching mid-session still sticks.

The sidebar toggle exists because Manage Connections needs MANAGE_CONNECTIONS to open at all; without it a Developer or Data Engineer, who typically holds exactly one granted connection, would have no way to set a default.

One thing reviewers should look at

ConnectionScopedAuthorizationSafetyTest now flags GET /connections, because the handler resolves the caller's pinned connection id and the scanner matches (?i)connection_?id anywhere in a handler body. That endpoint takes no arguments at all — it returns whatever getConnectionsForUser(username, isAdmin) gives. It is exempted in AUTHORIZED_ELSEWHERE, and a new connectionListingTakesNoCallerSuppliedId test re-derives the claim so the exemption cannot rot into cover for a real gap. The scanner was not weakened and no meaningless assert was added.

Verified against a live backend

On a throwaway Postgres/backend stack, not inferred:

• ddl-auto creates connection_pin with its unique index on boot
• pinning a second connection flips the first to false
• unpinning a non-pinned connection leaves the real pin alone
• deleting a connection clears its pin row
• a DEVELOPER with no grant gets 403, not 500
• a DEVELOPER holding only a grant (canManageConfig: false) pins successfully, and their pin does not appear on the admin's list

---

2. fix: Editor reported malformed SQL as a permissions denial

Reported from the field. A user pasted a SELECT that still carried the double quotes it had in source code ("select h.id, ...) and was told "Only admins can execute DDL or DML from the SQL Editor" — which reads as a permissions problem and sent people looking for a role fix.

The statement is neither DDL nor DML. With an unclosed " the whole thing is one quoted identifier, so it is not valid SQL at all.

Two keyword heuristics disagreed, and the disagreement was resolved as "mutation":

detectQueryType = SELECT      isReadOnlyQuery = false      parse = FAILED

QueryNormalizer.detectQueryType sanitizes a prefix away and answers SELECT; the provider's isReadOnlyQuery strips only comments, still sees the leading quote, and answers false. mutating = !readOnly && type != UNKNOWN then labelled a SELECT a mutation.

classifyStatement now records that the parser rejected the statement and, when the detected verb is read-only and no hidden write was found, returns notParseable; enforce throws STATEMENT_NOT_PARSEABLE ahead of both the read-only and confirmation branches, with a message naming the likely cause.

This does not weaken the guard

The statement is still blocked, admins included — only the diagnosis changed. An admin is deliberately not offered a confirmation prompt for a statement nothing managed to classify, since confirming past the guard is the one way this could become a bypass.

The reclassification is gated on isReadOnlyVerb(queryType) and hiddenWrite == null. Both halves are tested:

• anUnparseableWriteIsStillTreatedAsAMutation — DELETE FROM hotel WHERE ((( still returns EDITOR_MUTATION_FORBIDDEN
• aMalformedDataModifyingCteIsStillBlockedAsAWrite — a broken WITH x AS (DELETE ...) is still a blocked write, not a reported typo

The MCP guard already reported this case honestly ("Only read-only SQL is allowed …") and was left alone.

---

Testing

• QueryExecutionPolicyServiceTest: 49 pass (43 before, 6 added), including every pre-existing guard case — data-modifying CTEs, SELECT INTO, DROP blocking, EXPLAIN-wrapped writes.
• Full backend suite diffed against pristine HEAD: 1531 vs 1512 tests, failure sets byte-identical. The 99 failures are pre-existing environmental context-load failures (no DB/Redis in the test container), unchanged by this branch.
• Commit 1 built and tested in isolation in a detached worktree, so it is bisect-safe rather than only green as part of the combined tree.
• Frontend: npm run build clean; 0 lint errors in changed files.

🤖 Generated with Claude Code