diff --git a/internal/gateway/apiv1.go b/internal/gateway/apiv1.go index 2f601ea..363f9ab 100644 --- a/internal/gateway/apiv1.go +++ b/internal/gateway/apiv1.go @@ -68,6 +68,19 @@ func (g *Gateway) apiV1Routes(w http.ResponseWriter, r *http.Request) { } for _, s := range g.core.Sources() { if s.Name == name { + // ?reveal=credentials is admin-only and is what the Web UI's + // edit dialog uses: a form that round-trips a source must be + // able to show the current key, otherwise saving an unrelated + // field would blank it. Everything else stays masked. + if r.URL.Query().Get("reveal") == "credentials" { + if reqRole(r.Context()) != "admin" { + writeError(w, http.StatusForbidden, "forbidden", + "admin role required to reveal credentials") + return + } + writeJSON(w, http.StatusOK, map[string]interface{}{"source": s}) + return + } writeJSON(w, http.StatusOK, map[string]interface{}{"source": maskSource(s)}) return } @@ -176,6 +189,9 @@ func (g *Gateway) apiV1Index(w http.ResponseWriter, r *http.Request) { "path_escape": "URL-encode source and key names; {name} is a single path segment", "idempotency": "POST /api/sources and PUT /api/sources/{name} both upsert by name", "config_truth": "all configuration lives in config.yaml; API writes are persisted immediately", + "credentials": "credentials are masked by default. GET /api/v1/sources/{name}?reveal=credentials " + + "returns them in the clear and is admin-only — the Web UI edit dialog uses it, because a form " + + "that round-trips a source must show the current key or saving another field would blank it.", }, } writeJSON(w, http.StatusOK, index) diff --git a/internal/gateway/apiv1_test.go b/internal/gateway/apiv1_test.go index b190ab9..c688ef5 100644 --- a/internal/gateway/apiv1_test.go +++ b/internal/gateway/apiv1_test.go @@ -271,3 +271,29 @@ func serveViaHandler(t *testing.T, g *Gateway, req *http.Request) *httptest.Resp g.Handler().ServeHTTP(rec, req) return rec } + +// TestAPIV1RevealRequiresAdmin: the only way to read a key in the clear is the +// explicit admin-only reveal. Without the role check this endpoint would hand +// every source credential to any valid (even user-scoped) key. +func TestAPIV1RevealRequiresAdmin(t *testing.T) { + g, _, user := v1Gateway(t) + rec := serveViaHandler(t, g, newAuthedRequest(t, http.MethodGet, "/api/v1/sources/up?reveal=credentials", user)) + if rec.Code != http.StatusForbidden { + t.Errorf("user reveal = %d, want 403: %s", rec.Code, rec.Body.String()) + } + if strings.Contains(rec.Body.String(), "sk-up-secret") { + t.Error("a forbidden reveal still leaked the key") + } + rec = doReq(t, g, http.MethodGet, "/api/v1/sources/up?reveal=credentials", "") + if rec.Code != http.StatusOK { + t.Fatalf("admin reveal = %d: %s", rec.Code, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "sk-up-secret") { + t.Error("admin reveal should return the key in the clear (the edit form needs it)") + } + // The masked default view must still hide it on the same path. + rec = doReq(t, g, http.MethodGet, "/api/v1/sources/up", "") + if strings.Contains(rec.Body.String(), "sk-up-secret") { + t.Error("the default view leaked the key") + } +} diff --git a/internal/gateway/ui/index.html b/internal/gateway/ui/index.html index e054a05..94704de 100644 --- a/internal/gateway/ui/index.html +++ b/internal/gateway/ui/index.html @@ -2772,7 +2772,10 @@ /* ---------- sources tab ---------- */ async function renderSources() { - const j = await api("/api/sources"); + // Read through /api/v1: the UI is a client of the same public API an + // agent uses, so there is one contract rather than a private path. + // The list view needs no credentials, so the masked view is enough. + const j = await api("/api/v1/sources"); const rows = j.sources .map( ( @@ -2794,13 +2797,23 @@ function editSource(name) { const wrap = document.createElement("div"); wrap.id = "modal-wrap"; - Promise.all([api("/api/sources"), api("/api/status")]) - .then(([src, st]) => { - const s = src.sources.find((x) => x.name === name) || { - name: name, - models: [{ id: "", priority: 0, kind: "chat" }], - }; - const cur = s.adapter || "openai"; + // The edit dialog round-trips the whole source, so it must load the + // real api_key — otherwise saving an unrelated field would blank + // it. That is what ?reveal=credentials is for (admin-only). + const srcPath = name + ? "/api/v1/sources/" + encodeURIComponent(name) + "?reveal=credentials" + : "/api/v1/sources"; + Promise.all([api(srcPath), api("/api/status")]) + .then(([src, st]) => { + // Editing: the single-source shape. Creating: an empty form. + const s = name + ? src.source + : { name: "", models: [{ id: "", priority: 0, kind: "chat" }] }; + if (!s) { + toast(t("toastSave")); + return; + } + const cur = s.adapter || "openai"; const apps = ["", ...(st.adapters || []).map((a) => a.name)]; if (cur && !apps.includes(cur)) apps.push(cur); const adSel = @@ -3215,7 +3228,7 @@ const sortLanes = { chat: [], image: [] }; let sortStateMap = new Map(); async function renderSort() { - const j = await api("/api/sources"); + const j = await api("/api/v1/sources"); let autoR = []; let autoImg = []; try { @@ -4155,7 +4168,7 @@ async function loadModelPairs() { let srcs = []; try { - srcs = (await api("/api/sources")).sources || []; + srcs = (await api("/api/v1/sources")).sources || []; } catch (e) {} const pairs = []; (srcs || []).forEach((s) =>