VS Code's sign-in keeps its place; recordings checked for secrets
CI / test (push) Successful in 9m14s
CI / test (push) Successful in 9m14s
From review. - /profile asked before signing in keeps its query for ten minutes, and the sign-in (password or OIDC) returns to it: VS Code's sign-in state came back lost, and the confirmation came every time. Only /profile is ever returned to. - The extension keeps an https gateway at https when a link names it by plain http (a proxy ending TLS unannounced), so the key is never sent in the clear. The install command says --force, since dev builds share a version and VS Code kept the old one. - Every frame of every install recording is checked for a key, a password or a private key, not only the page. - The admin API document again matches the API: f94d902's MCPServerRequest.clear was not in it. - CHANGELOG for this round's fixes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
258591b3f2
commit
ecf5545a7a
11 files changed
+133
-6
No files matched your search
@@ -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.
|
||||
|
||||
@@ -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\"."
|
||||
}
|
||||
}
|
||||
},
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
@@ -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);
|
||||
});
|
||||
+31
-1
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
@@ -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},
|
||||
|
||||
@@ -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") {
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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.
|
||||
|
||||
Reference in new issue
Block a user