Storage/Cluster Admin Secret Masking — Plan

Goal

Fix a real bug found in production use: editing the Default Storage Backend in Admin > Storage and clicking "Test Connection" fails with InvalidAccessKeyId, even though the credentials stored in the database are correct and MinIO is reachable (confirmed directly with the MinIO Python SDK against the stored minioadmin/minioadmin credentials — they work).

Root cause: StorageBackend.config (and, identically, Cluster.provider_config / Cluster.registry_auth) is deliberately write-only — the admin API never sends secret values back to the browser (see storage-admin-ui.md Design decisions: "Secrets never round-trip to the browser"). The edit form therefore always starts with blank fields for these values. Blank is indistinguishable from "leave unchanged": clicking Test Connection (or Save, if any field in the config group was touched) sends the blank fields as-is, which for MinIO means an empty access key/secret key — MinIO correctly rejects that as InvalidAccessKeyId.

This plan replaces "blank on edit" with "send back a **** placeholder per field, and resolve **** back to the stored value server-side on Save/Test Connection." Secrets still never actually round-trip to the browser in plaintext — only the fixed placeholder string does — so the no-round-trip property from storage-admin-ui.md is preserved, while fixing the actual bug: leaving a field untouched during edit now genuinely means "unchanged," for both Save and Test Connection.

Confirmed with the user that the identical bug pattern exists in Admin > Clusters (Cluster.provider_config, e.g. pasted kubeconfig; Cluster.registry_auth), since it's built on the exact same configTouched/blank-on-edit mechanism as storage (cluster-admin-ui.md). This plan fixes both in one pass, since it's the same fix applied to two structurally identical call sites, not two different designs.

Also removes has_config/has_provider_config/has_registry_auth and the "(currently set — enter to replace)" placeholder-text mechanism in the same pass: once a set field is pre-filled with the real **** placeholder content (not left blank), that boolean and its placeholder-text convention become dead weight — the masked value itself already conveys "this is set." Confirmed nothing else in the frontend reads these has_* flags (only the form-modal placeholder logic did).

Supersedes / modifies

Modifies the "Secrets never round-trip to the browser" design decision in storage-admin-ui.md and the equivalent one in cluster-admin-ui.md: the no-plaintext-round-trip property is kept, but "blank on edit" becomes "**** placeholder on edit, resolved server-side." Does not change credential_strategy, mint()/provision(), or anything about which protocols/cluster types are implemented.

Background — current state (confirmed by reading the code)

Design decisions (settled in discussion)

Phase 1 — Shared masking helpers

1.1 backend/services/secret_masking.py (new file)

"""Shared helpers for write-only secret fields in the admin API (StorageBackend.config,
Cluster.provider_config, Cluster.registry_auth). Secrets are never sent to the browser in
plaintext — GET/list responses substitute MASKED for each set field. On Save/Test Connection, any
field still equal to MASKED means "leave unchanged" and is resolved back to the stored value here,
rather than being persisted (or tested against) literally."""

MASKED = "****"


def mask_config(config):
    return {k: MASKED for k in (config or {})}


def mask_secret(value):
    return MASKED if value else None


def resolve_config(submitted, stored):
    stored = stored or {}
    resolved = dict(submitted or {})
    for key, value in resolved.items():
        if value == MASKED:
            if key not in stored:
                raise ValueError(
                    f"cannot restore masked value for {key!r}: no existing value stored"
                )
            resolved[key] = stored[key]
    return resolved


def resolve_secret(submitted, stored):
    if submitted == MASKED:
        if not stored:
            raise ValueError("cannot restore masked value: no existing value stored")
        return stored
    return submitted

No test-writing infrastructure exists elsewhere in backend/services/, so no new test file is added here — consistent with the rest of the codebase (see CLAUDE.md Testing section: backend tests are a TODO).

Phase 2 — Storage backend routes (backend/routers/admin.py)

2.1 _storage_backend_admin_dict

def _storage_backend_admin_dict(backend: StorageBackend) -> Dict:
    d = backend.to_dict()
    d["config"] = mask_config(backend.config)
    return d

(drops the has_config line)

2.2 _test_and_apply_storage_connection (PATCH path)

async def _test_and_apply_storage_connection(backend: StorageBackend, body: Dict) -> None:
    if "protocol" in body or "config" in body:
        protocol = body.get("protocol", backend.protocol)
        submitted = body.get("config") or {}
        stored = backend.config if protocol == backend.protocol else {}
        try:
            config = resolve_config(submitted, stored)
            handler = get_protocol_handler(protocol)
            await handler.test_connection(_TestBackend(backend.endpoint, config))
        except HTTPException:
            raise
        except ValueError as e:
            raise HTTPException(status_code=400, detail=str(e))
        except Exception as e:
            raise HTTPException(status_code=400, detail=f"Connection test failed: {e}")
        backend.protocol = protocol
        backend.config = config

Ordering is unchanged (still runs after _apply_storage_generic_fields, still reads backend.config — the pre-update stored value — before overwriting it), and the create path (admin_create_storage_backend) goes through the same function unchanged; a brand-new backend has no stored config, so a stray "****" there correctly raises rather than silently doing nothing.

2.3 Stateless test-connection route

@router.post("/admin/storage-backends/test-connection")
async def admin_test_storage_backend_connection(
    body: Dict, auth=Depends(require_admin), db: AsyncSession = Depends(get_db)
):
    protocol = body.get("protocol")
    if not protocol:
        raise HTTPException(status_code=400, detail="protocol is required")
    stored = {}
    backend_id = body.get("backend_id")
    if backend_id:
        existing = await db.get(StorageBackend, backend_id)
        if existing is not None and existing.protocol == protocol:
            stored = existing.config or {}
    try:
        config = resolve_config(body.get("config") or {}, stored)
        handler = get_protocol_handler(protocol)
        await handler.test_connection(_TestBackend(body.get("endpoint"), config))
    except ValueError as e:
        raise HTTPException(status_code=400, detail=str(e))
    except Exception as e:
        raise HTTPException(status_code=400, detail=f"Connection test failed: {e}")
    return {"ok": True}

Phase 3 — Storage frontend (StorageBackendsAdminPanel.jsx + protocol forms)

Phase 4 — Cluster routes (backend/routers/admin.py)

Mirrors Phase 2 exactly:

Phase 5 — Cluster frontend (ClustersAdminPanel.jsx + provider forms)

Open Questions