# Patch Analysis for CVE-2026-59826 Variant Review

## What the fix changes

The fixing commit identified from the parent reproduction is `74032e5e0a5a70dc45a6a744d37b9ba24eee8d01` (`v1.61.2`). It changes the database-routing destination creation flow and the driver validation interface.

Relevant files/functions:

1. `src/metabase/driver.clj`
   - Adds the `driver/validate-db-details!` multimethod.
   - The method is intended to reject database details that are unsafe to persist, independent of whether the database is reachable.
   - The default implementation is a no-op, so only drivers with explicit unsafe persistence rules need custom methods.

2. `src/metabase/driver/h2.clj`
   - Moves the existing H2 unsafe-detail checks out of `driver/can-connect? :h2` into `driver/validate-db-details! :h2`.
   - `validate-db-details! :h2` rejects H2 data-warehouse use unless `driver.settings/*allow-testing-h2-connections*` is true.
   - When a `db` string is present, it parses the H2 JDBC connection string with `org.h2.engine.ConnectionInfo` and rejects:
     - `INIT` connection properties; and
     - property values containing known dangerous markers, including semicolons and scripting language markers.
   - `driver/can-connect? :h2` now calls `driver/validate-db-details!` before opening the connection.

3. `enterprise/backend/src/metabase_enterprise/database_routing/api.clj`
   - Imports `metabase.driver`.
   - Changes `POST /api/ee/database-routing/destination-database` so it always validates each destination's details with `driver/validate-db-details!` before persistence.
   - Retains optional reachability testing through `check_connection_details`, but separates unsafe-detail validation from reachability. This is important because destination databases are allowed to be unreachable at creation time.
   - If validation throws, the endpoint returns HTTP 400 with an error body keyed by destination name and does not insert the destination `:model/Database` row.

4. `src/metabase/driver/mysql.clj`
   - Moves MySQL unsafe additional-options checks into `driver/validate-db-details! :mysql`, so they can also be applied to persistence-only paths.

## What assumptions the fix makes

The patch assumes:

- The unsafe path requiring a new validation-only hook is `POST /api/ee/database-routing/destination-database`, because that endpoint deliberately does not test reachability before persisting destination details.
- Other user/admin-controlled database-detail entry points either already test connectivity or have their own guard before persistence.
- For H2, rejecting the driver as a data warehouse and rejecting malicious parsed connection properties is sufficient to block the unsafe H2 execution primitive at database-detail persistence/open time.
- It is acceptable for internal/sample-database flows to continue using dynamic bindings such as `*allow-testing-h2-connections*` when they need to open bundled H2 databases for product operation or tests.

## Target threat model scope

The repository's `SECURITY.md` says security issues should be responsibly disclosed to `security@metabase.com`; it does not exclude administrator-supplied remote API inputs from scope. The parent issue crosses a real trust boundary: an authenticated Metabase administrator sends crafted connection details to the Metabase server over a remote HTTP API, and the server later opens those details inside the Metabase JVM. The variant review therefore stayed within the same trust boundary: authenticated remote/admin API or import surfaces that accept database details and persist or open them server-side.

The review did not treat local filesystem edits, direct app-database writes by the server operator, or normal H2 file loading chosen by the local operator as security variants.

## What code paths/inputs the fix does NOT directly cover

The patch does not install a global `:model/Database` before-insert/before-update validation hook. Therefore, in principle, any other code path that directly inserts or updates `:model/Database :details` from administrator-controlled input without connection testing or an H2-specific guard would remain a possible variant.

Reviewed candidate paths:

1. `POST /api/database` and `PUT /api/database/:id`
   - These are not directly modified by the patch.
   - They already call `test-connection-details` or `test-database-connection`, which reaches `driver/can-connect? :h2` and therefore `driver/validate-db-details! :h2` after the patch. Runtime testing confirmed both v1.61.1 and v1.61.2 reject the malicious H2 details with `H2 is not supported as a data warehouse`.

2. Serialization import (`POST /api/ee/serialization/import`)
   - Not directly modified by the database-routing patch.
   - `src/metabase/warehouses/models/database.clj` defines `assert-not-h2!` and calls it from `serdes/load-one! "Database"`, rejecting imported H2 database entities before the default load path persists them. Source inspection shows this guard exists in both the vulnerable and fixed tested tags.
   - Runtime import probing in the product images failed before importing the crafted archive and did not create marker files.

3. Advanced config database file loader
   - Not directly modified by the database-routing patch.
   - Source in `enterprise/backend/src/metabase_enterprise/advanced_config/file/databases.clj` calls `driver.u/can-connect-with-details?` before creating/updating non-deletion database entries, which reaches the H2 validation path.
   - The later runtime upload endpoint `/api/ee/advanced-config/` was not present in the tested v1.61.1/v1.61.2 images and returned 404 in both images.

4. Internal/sample database and sync paths
   - Some internal paths intentionally bind `*allow-testing-h2-connections*` for bundled sample database operations or sync. These are not considered variants because they are not distinct remote/admin persistence paths for attacker-supplied H2 connection details across the same trust boundary.

## Whether the fix is complete or leaves gaps

No fixed-version bypass was confirmed. For the tested product images and reviewed code paths, the fix appears complete for the parent trust boundary and sink:

- The vulnerable database-routing destination endpoint now rejects unsafe H2 details before persistence.
- The normal database API rejects unsafe H2 details through connection validation.
- Serialization import has an independent H2 database import guard.
- Runtime advanced-config upload was unavailable in the tested images, and the config-file loader source path tests connectivity before database creation/update.

A defense-in-depth gap remains architectural rather than currently exploitable in this run: unsafe database-detail persistence is not centralized. If a future feature directly writes `:model/Database :details` from remote/admin input without connection testing, it could recreate the same class of bug. A central model-level or service-level validation boundary would make this invariant harder to bypass accidentally.

## Behavior before and after the fix

Parent database-routing path:

- Before fix (`v1.61.1`): `POST /api/ee/database-routing/destination-database?check_connection_details=false` persists the malicious H2 destination details, and a later `POST /api/dataset` opens the destination and executes attacker-controlled H2 SQL.
- After fix (`v1.61.2`): the same destination creation request is rejected with HTTP 400 before persistence.

Variant candidates tested in this stage:

- `POST /api/database`: rejected in both `v1.61.1` and `v1.61.2` with `H2 is not supported as a data warehouse`; no marker file was created.
- `POST /api/ee/serialization/import`: did not import the crafted H2 database in either image; source review shows a pre-existing `assert-not-h2!` import guard.
- `/api/ee/advanced-config/`: returned HTTP 404 in both tested images; no runtime upload path was available to exercise.

## Variant/rule-out matrix

| Candidate | Entry point | Same sink? | Runtime result | Fixed-version result | Outcome |
|---|---|---:|---|---|---|
| Normal database creation | `POST /api/database` | Yes, would persist/open H2 `details.db` if accepted | HTTP 400 in v1.61.1 | HTTP 400 in v1.61.2 | Ruled out; already validated |
| Serialization import | `POST /api/ee/serialization/import` | Potentially, if imported Database were persisted | Import failed before persistence; source has `assert-not-h2!` | Same | Ruled out; independent H2 guard |
| Runtime advanced-config upload | `POST /api/ee/advanced-config/` | Potentially, if endpoint existed and created DBs | HTTP 404 | HTTP 404 | Not available in tested target; source loader tests connection |

## Recommendation

Keep the current patch and add regression coverage around all database-detail write surfaces. For stronger long-term safety, introduce a centralized `validate-db-details!` enforcement point in the service/model layer used by any external/admin-controlled `:model/Database` insertion or update, with explicit internal bypasses only for trusted sample database and maintenance flows.