diff --git a/integration/integration_test.go b/integration/integration_test.go index 678f491..937c0e0 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -48,6 +48,15 @@ const ( // much lower per-minute bucket so that loop's volume can never // masquerade as a reload-caused failure via a budget-exhaustion 429. hotReloadProbeKey = "sk-int-hotreload" + // daveKey belongs to the dedicated "restricted" group (models: + // ["gpt-mock"], passthroughPaths: ["v1/chat/completions"], + // dynamic.yml.tmpl) — used only by + // TestPassthroughModelAndPathRestriction (security review finding + // 2/3, round 3, 2026-08-22 report correction) to exercise the + // native-passthrough model-peek and path-allowlist enforcement under + // real Yaegi interpretation; every other group/user in this suite + // leaves both fields unset. + daveKey = "sk-int-dave" ) // composeFile returns the path to docker-compose.yml relative to this @@ -657,6 +666,52 @@ func TestPassthroughAndMCPStream(t *testing.T) { } } +// TestPassthroughModelAndPathRestriction (security review finding 2/3, +// round 3, 2026-08-22 — closes a report-correction gap, not a code +// defect: the reviewer verified peekPassthroughModel/scanTopLevelModel +// and allowsPassthroughPath directly via the repo's own Yaegi harness, +// but every group/user this suite drove native passthrough through left +// models/passthroughPaths unset, so hasModelRestriction() was always +// false and this suite itself never exercised either enforcement path +// under a real Traefik+Yaegi process) drives dave (the "restricted" +// group: models: ["gpt-mock"], passthroughPaths: ["v1/chat/completions"]) +// through native passthrough three ways: an allowed model on the +// allowed path (200, proxied for real), a denied model on that SAME +// allowed path (403 — proves the model peek runs and rejects), and an +// ALLOWED model on a path outside the allowlist (403 — proves +// allowsPassthroughPath's non-empty-list branch runs and rejects; the +// model on this third request is deliberately the allowed one, so a +// pass here can only be explained by the path check, not the model +// check, having fired). +func TestPassthroughModelAndPathRestriction(t *testing.T) { + allowedBody := map[string]any{ + "model": "gpt-mock", + "messages": []map[string]any{{"role": "user", "content": "hi"}}, + } + resp, body := doJSON(t, http.MethodPost, traefik1URL+"/openai/v1/chat/completions", daveKey, allowedBody) + if resp.StatusCode != http.StatusOK { + t.Fatalf("allowed model+path: status = %d, body=%#v", resp.StatusCode, body) + } + + deniedModelBody := map[string]any{ + "model": "not-in-the-allowlist", + "messages": []map[string]any{{"role": "user", "content": "hi"}}, + } + resp, body = doJSON(t, http.MethodPost, traefik1URL+"/openai/v1/chat/completions", daveKey, deniedModelBody) + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("denied model, allowed path: status = %d, want 403, body=%#v", resp.StatusCode, body) + } + + // "v1/embeddings" is not in passthroughPaths (["v1/chat/completions"]); + // the model ("gpt-mock") is the allowed one, so a 403 here can only + // come from the path check, not the model check. + deniedPathBody := map[string]any{"model": "gpt-mock", "input": "hi"} + resp, body = doJSON(t, http.MethodPost, traefik1URL+"/openai/v1/embeddings", daveKey, deniedPathBody) + if resp.StatusCode != http.StatusForbidden { + t.Fatalf("allowed model, denied path: status = %d, want 403, body=%#v", resp.StatusCode, body) + } +} + // TestFederatedMCP covers integration test 8 (SF4, review round 2, v0.21): // POST /mcp aggregates both the lenient "tool" server (a bare tools/list // answers outright) and the strict "toolstrict" server (rejects a bare diff --git a/integration/traefik/dynamic.yml.tmpl b/integration/traefik/dynamic.yml.tmpl index 303ef33..12add8f 100644 --- a/integration/traefik/dynamic.yml.tmpl +++ b/integration/traefik/dynamic.yml.tmpl @@ -80,6 +80,22 @@ http: hotreload: limits: requestsPerMinute: 100000 + # Dedicated to TestPassthroughModelAndPathRestriction + # (security review finding 2/3, round 3, 2026-08-22 report + # correction): every OTHER group in this file leaves models + # and passthroughPaths unset, so hasModelRestriction() is + # always false and allowsPassthroughPath's non-empty-list + # branch is never reached — meaning the native-passthrough + # model-peek (peekPassthroughModel/scanTopLevelModel) and + # path-allowlist (allowsPassthroughPath) code paths were + # NEVER exercised under real Yaegi interpretation by this + # suite, contrary to what an earlier report claimed. This + # group exists specifically to close that gap. + restricted: + limits: + requestsPerMinute: 1000 + models: ["gpt-mock"] + passthroughPaths: ["v1/chat/completions"] users: inline: # limits.requestsPerDay is set (generously — every test in @@ -114,6 +130,12 @@ http: - name: hotreload-probe group: hotreload apiKey: "sk-int-hotreload" + # TestPassthroughModelAndPathRestriction's dedicated + # identity — see the "restricted" group's own comment + # above. + - name: dave + group: restricted + apiKey: "sk-int-dave" file: "/users/users.json" redis: address: "redis:6379"