diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bed9cc..ff6202c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,22 @@ APIs change between minors without a deprecation period. ### Fixed +- **VS Code's sign-in lost its place through the sign-in page**, so the + prompt to confirm came every time; a link to the same gateway by plain + http could take an https setting down to http; and a new build of the + extension was not installed over an old one (the command now says + `--force`). +- **The install guide's recordings were not checked for secrets**, only + the page; every frame is now. +- **MCP server fields could not be emptied**, an address could carry a + password, the cluster's own server could not be changed from the + console, and a grant to a key that does not exist was said done. +- **A guardrail service two policies named was asked as the first said**, + not the stricter; a `null` answer read as safe; and with `restore` the + request log kept the answer with the redacted values back in it. +- **What an engine downloads by a Hugging Face id was not held to the + pickle refusal** — an adapter by its id, a TensorRT-LLM model's; a model + with either now needs `allow_pickle`. - **`server init` run again on a machine already set up was refused its memory**, as on an 8 GB install fixing its certificate with `--host`, and judges that would not be installed (no mirror) were counted. diff --git a/docs/admin-api.openapi.json b/docs/admin-api.openapi.json index 76d36f4..d0926f5 100644 --- a/docs/admin-api.openapi.json +++ b/docs/admin-api.openapi.json @@ -7550,6 +7550,13 @@ "rate_per_minute": { "type": "integer", "description": "RatePerMinute bounds each caller's calls to its tools; a negative number takes the bound away." + }, + "clear": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Clear empties what an empty field above would leave as it is: \"description\", \"timeout\" (back to the default), \"rate\"." } } }, diff --git a/internal/vscodeext/ext/extension.js b/internal/vscodeext/ext/extension.js index b1141da..1d3a7ee 100644 --- a/internal/vscodeext/ext/extension.js +++ b/internal/vscodeext/ext/extension.js @@ -103,12 +103,18 @@ function activate(context) { async function handleUri(uri) { if (uri.path !== '/connect') return; const q = new URLSearchParams(uri.query); - const base = lib.gatewayBase(q.get('gateway')); + let base = lib.gatewayBase(q.get('gateway')); const key = (q.get('key') || '').trim(); if (!base || !key) { vscode.window.showErrorMessage('llmgw: the link has no gateway or no key in it.'); return; } + // The same gateway by plain http, behind a proxy that ends TLS without + // saying so: kept at https, so the key is never sent in the clear. + const current = gateway(); + if (current && lib.downgrades(current, base)) { + base = current; + } const expected = pending && pending.base === base && q.get('state') === pending.state && Date.now() - pending.at < SIGN_IN_TTL_MS; pending = null; diff --git a/internal/vscodeext/ext/lib.js b/internal/vscodeext/ext/lib.js index 93343f2..45efe9d 100644 --- a/internal/vscodeext/ext/lib.js +++ b/internal/vscodeext/ext/lib.js @@ -263,7 +263,19 @@ function tokenGuess(text) { return Math.ceil(String(text || '').length / 4); } +// downgrades reports whether next is the gateway at current by plain http +// where current is https: the same host and port, a weaker scheme. +function downgrades(current, next) { + try { + const a = new URL(current); + const b = new URL(next); + return a.protocol === 'https:' && b.protocol === 'http:' && a.hostname === b.hostname; + } catch (e) { + return false; + } +} + module.exports = { USER, ASSISTANT, SYSTEM, - gatewayBase, modelInfo, chatMessages, chatTools, requestBody, sseData, Answer, errorMessage, tokenGuess, + gatewayBase, modelInfo, chatMessages, chatTools, requestBody, sseData, Answer, errorMessage, tokenGuess, downgrades, }; diff --git a/internal/vscodeext/ext/lib_test.js b/internal/vscodeext/ext/lib_test.js index b61c029..f6dfc60 100644 --- a/internal/vscodeext/ext/lib_test.js +++ b/internal/vscodeext/ext/lib_test.js @@ -121,3 +121,10 @@ test('errorMessage says what to do about a refused key', () => { assert.match(lib.errorMessage(429, '{"error":{"message":"rate limit"}}'), /rate limit/); assert.match(lib.errorMessage(500, 'boom'), /500: boom/); }); + +test('a link to the same gateway by plain http is a downgrade', () => { + assert.equal(lib.downgrades('https://llm.example.com', 'http://llm.example.com'), true); + assert.equal(lib.downgrades('https://llm.example.com', 'https://llm.example.com'), false); + assert.equal(lib.downgrades('http://llm.example.com', 'https://llm.example.com'), false); + assert.equal(lib.downgrades('https://a.example.com', 'http://b.example.com'), false); +}); diff --git a/internal/web/account.go b/internal/web/account.go index 4f9fe82..5a0d852 100644 --- a/internal/web/account.go +++ b/internal/web/account.go @@ -8,6 +8,7 @@ import ( "errors" "log/slog" "net/http" + "net/url" "strconv" "strings" "sync" @@ -339,7 +340,7 @@ func (p *AccountPages) login(w http.ResponseWriter, r *http.Request) { setSessionCookie(w, r, token) setCSRFCookie(w, r, csrf) - http.Redirect(w, r, "/profile", http.StatusSeeOther) + http.Redirect(w, r, afterSignIn(w, r), http.StatusSeeOther) } func (p *AccountPages) logout(w http.ResponseWriter, r *http.Request) { @@ -430,6 +431,12 @@ func (p *AccountPages) profile(w http.ResponseWriter, r *http.Request) { noStore(w) sess, user, ok := p.session(r) if !ok { + // Back here after signing in, with what was asked: VS Code's + // sign-in carries its state in the query, and lost it otherwise. + if r.URL.RawQuery != "" { + http.SetCookie(w, &http.Cookie{Name: afterSignInCookie, Value: url.QueryEscape(r.URL.RawQuery), + Path: "/", HttpOnly: true, Secure: isHTTPS(r), SameSite: http.SameSiteLaxMode, MaxAge: 600}) + } http.Redirect(w, r, "/login", http.StatusSeeOther) return } @@ -827,3 +834,26 @@ func errorRef() string { // SetHistory gives the pages the metrics store. func (p *AccountPages) SetHistory(h History) { p.history = h } + +// afterSignInCookie holds, for ten minutes, the query /profile was asked +// with before signing in. +const afterSignInCookie = "llmgw_after" + +// afterSignIn is where a sign-in goes: /profile, with the query it was +// first asked with — only ever /profile, so the cookie cannot send anyone +// elsewhere. +func afterSignIn(w http.ResponseWriter, r *http.Request) string { + c, err := r.Cookie(afterSignInCookie) + if err != nil { + return "/profile" + } + http.SetCookie(w, &http.Cookie{Name: afterSignInCookie, Path: "/", MaxAge: -1}) + q, err := url.QueryUnescape(c.Value) + if err != nil { + return "/profile" + } + if _, err := url.ParseQuery(q); err != nil || strings.ContainsAny(q, "\r\n") { + return "/profile" + } + return "/profile?" + q +} diff --git a/internal/web/admin_mcp_test.go b/internal/web/admin_mcp_test.go index 9b08c4e..3dd5ec0 100644 --- a/internal/web/admin_mcp_test.go +++ b/internal/web/admin_mcp_test.go @@ -207,3 +207,28 @@ func TestAnMCPServersFieldsCanBeEmptiedAndItsAddressHoldsNoPassword(t *testing.T t.Errorf("a grant to no key: %s", body) } } + +func TestASignInComesBackToWhatProfileWasAskedWith(t *testing.T) { + // VS Code's sign-in opens /profile?tool=vscode&state=…; not signed in, + // that went to /login and came back to a bare /profile. + p := NewAccountPages(fakeAccounts{}, nil, nil, "v0", slog.New(slog.NewTextHandler(io.Discard, nil))) + w := httptest.NewRecorder() + p.profile(w, httptest.NewRequest("GET", "/profile?tool=vscode&state=s123", nil)) + var kept *http.Cookie + for _, c := range w.Result().Cookies() { + if c.Name == afterSignInCookie { + kept = c + } + } + if w.Code != http.StatusSeeOther || kept == nil { + t.Fatalf("%d, cookie %v", w.Code, kept) + } + r := httptest.NewRequest("POST", "/login", nil) + r.AddCookie(kept) + if got := afterSignIn(httptest.NewRecorder(), r); got != "/profile?tool=vscode&state=s123" { + t.Errorf("after signing in: %q", got) + } + if got := afterSignIn(httptest.NewRecorder(), httptest.NewRequest("POST", "/login", nil)); got != "/profile" { + t.Errorf("without one: %q", got) + } +} diff --git a/internal/web/connect.go b/internal/web/connect.go index cfcbafd..484b536 100644 --- a/internal/web/connect.go +++ b/internal/web/connect.go @@ -149,7 +149,7 @@ func connectIn(l *Lang, tool, base, model, key string, f ModelFacts) connectView v.Notes = []string{l.T("connect.note.codex_blocks"), l.T("connect.note.codex_env")} case "vscode": download := base + "/setup/vscode/" + vscodeext.FileName - v.Command = fmt.Sprintf("curl -fsSLo %[1]s '%[2]s' && code --install-extension %[1]s", vscodeext.FileName, download) + v.Command = fmt.Sprintf("curl -fsSLo %[1]s '%[2]s' && code --install-extension %[1]s --force", vscodeext.FileName, download) v.Link = vscodeLink(base, key, "") v.Fields = []connectField{ {l.T("connect.field.gateway"), base}, diff --git a/internal/web/connect_test.go b/internal/web/connect_test.go index d904e93..b3830e1 100644 --- a/internal/web/connect_test.go +++ b/internal/web/connect_test.go @@ -146,7 +146,7 @@ func TestContinueNeverAsksForMoreThanTheGatewayAllows(t *testing.T) { func TestVSCodeGetsTheExtensionAndALinkBack(t *testing.T) { c := connectFor("vscode", "https://gw.example", "m", "sk-llmgw-abc", ModelFacts{}) - if !strings.Contains(c.Command, "https://gw.example/setup/vscode/llmgw.vsix") || !strings.Contains(c.Command, "code --install-extension llmgw.vsix") { + if !strings.Contains(c.Command, "https://gw.example/setup/vscode/llmgw.vsix") || !strings.Contains(c.Command, "code --install-extension llmgw.vsix --force") { t.Errorf("command = %s", c.Command) } if strings.Contains(c.Command, "sk-llmgw-abc") { diff --git a/internal/web/install_test.go b/internal/web/install_test.go index a4366bb..5e71104 100644 --- a/internal/web/install_test.go +++ b/internal/web/install_test.go @@ -2,6 +2,7 @@ package web import ( "bytes" + "encoding/json" "net/http" "net/http/httptest" "regexp" @@ -104,3 +105,26 @@ func TestTheRecordingsAreServedCompressed(t *testing.T) { t.Errorf("an unknown recording answered %d", w.Code) } } + +func TestNoRecordingCarriesASecret(t *testing.T) { + // The page shows one screen of each; the player fetches every frame. + // Each frame as written, and as the player decodes it, is looked at. + secret := []*regexp.Regexp{ + regexp.MustCompile(`sk-llmgw-[A-Za-z0-9_-]{8,}`), + regexp.MustCompile(`(?i)(password|passphrase|token)[^\n]{0,40}?[:=]\s*[A-Za-z0-9+/_-]{12,}`), + regexp.MustCompile(`-----BEGIN [A-Z ]*PRIVATE KEY-----`), + } + for name, c := range guide.Casts { + for i, f := range c.Frames { + var text string + _ = json.Unmarshal(f, &text) + for _, s := range []string{string(f), text} { + for _, rx := range secret { + if m := rx.FindString(s); m != "" { + t.Errorf("recording %s frame %d shows %q", name, i, m) + } + } + } + } + } +} diff --git a/internal/web/oidc.go b/internal/web/oidc.go index f192a47..de68cb1 100644 --- a/internal/web/oidc.go +++ b/internal/web/oidc.go @@ -234,7 +234,7 @@ func (p *AccountPages) oidcCallback(w http.ResponseWriter, r *http.Request) { audit.Write(r.Context(), p.audit, p.log, store.AuditEvent{At: time.Now(), Actor: user.Email, Source: "web", Action: "sign in through " + o.cfg.Issuer, Status: http.StatusSeeOther, Detail: audit.Redact(map[string]any{"role": user.StaffRole()})}) - http.Redirect(w, r, "/profile", http.StatusSeeOther) + http.Redirect(w, r, afterSignIn(w, r), http.StatusSeeOther) } // errOIDCAccount is a provider's person the gateway will not let in, with why.