feat(mcp): add web.allowedorigins-removed rule; document v1.3.0 CORS migration
This commit is contained in:
@@ -49,10 +49,10 @@ func synthIndex() *index.Index {
|
||||
{Kind: "type", Name: "Config", SubPackage: "server", Fields: []index.Field{
|
||||
{Name: "Host", Type: "string", Tag: `env:"EINHERJAR_SERVER_HOST" envDefault:"0.0.0.0"`},
|
||||
{Name: "Port", Type: "int", Tag: `env:"EINHERJAR_SERVER_PORT" envDefault:"8080"`},
|
||||
{Name: "CORSOrigins", Type: "[]string", Tag: `env:"EINHERJAR_SERVER_CORS_ORIGINS" envSeparator:","`},
|
||||
}},
|
||||
{Kind: "type", Name: "Config", SubPackage: "", Fields: []index.Field{
|
||||
{Name: "Server", Type: "server.Config"}, // nested, no env tag
|
||||
{Name: "AllowedOrigins", Type: "[]string", Tag: `env:"EINHERJAR_SERVER_CORS_ORIGINS" envSeparator:","`},
|
||||
{Name: "Server", Type: "server.Config"}, // nested, no env tag; root has no own env vars
|
||||
}},
|
||||
},
|
||||
},
|
||||
@@ -73,7 +73,7 @@ func synthIndex() *index.Index {
|
||||
func TestForModule(t *testing.T) {
|
||||
idx := synthIndex()
|
||||
got := ForModule(idx, "web")
|
||||
if len(got) != 3 { // 2 server + 1 CORS; the nested Server field is not a leaf
|
||||
if len(got) != 3 { // Host + Port + CORSOrigins, all on server.Config; the nested Server field is not a leaf and the root has no own env vars
|
||||
t.Fatalf("ForModule(web) = %d vars, want 3: %+v", len(got), got)
|
||||
}
|
||||
names := map[string]bool{}
|
||||
@@ -93,15 +93,19 @@ func TestForModule(t *testing.T) {
|
||||
func TestFindStruct(t *testing.T) {
|
||||
idx := synthIndex()
|
||||
server := FindStruct(idx, "web", "server", "Config")
|
||||
if len(server) != 2 {
|
||||
t.Fatalf("FindStruct(web/server/Config) = %d, want 2", len(server))
|
||||
if len(server) != 3 {
|
||||
t.Fatalf("FindStruct(web/server/Config) = %d, want 3", len(server))
|
||||
}
|
||||
// The root web.Config must NOT be returned for the server selector.
|
||||
// CORS now lives on server.Config, so the server selector MUST include it.
|
||||
corsOnServer := false
|
||||
for _, v := range server {
|
||||
if v.Name == "EINHERJAR_SERVER_CORS_ORIGINS" {
|
||||
t.Error("server selector leaked the root web.Config CORS var")
|
||||
corsOnServer = true
|
||||
}
|
||||
}
|
||||
if !corsOnServer {
|
||||
t.Error("server selector should include EINHERJAR_SERVER_CORS_ORIGINS (it lives on server.Config)")
|
||||
}
|
||||
pg := FindStruct(idx, "db-postgres", "", "Config")
|
||||
if len(pg) != 3 {
|
||||
t.Fatalf("FindStruct(db-postgres//Config) = %d, want 3", len(pg))
|
||||
|
||||
@@ -206,6 +206,25 @@ appended, then feature hooks, then `lc.Run()`.
|
||||
Whichever tier you pick, CORS origins always come from the framework var
|
||||
`EINHERJAR_SERVER_CORS_ORIGINS` (`cfg.Server.CORSOrigins`) — never invent an app-owned CORS var.
|
||||
|
||||
> **CORS has one home: `server.Config.CORSOrigins`.** In framework `v1.x`, `web.Config`
|
||||
> carried an `AllowedOrigins` field. It was env-backed through `v1.1.x` and a code-only override
|
||||
> in `v1.2.0` — reading it after the env tag moved silently served *no* CORS. **`v2.0.0` removed the
|
||||
> field entirely** so the mistake fails at compile time instead of at runtime. If you are migrating
|
||||
> code that read `web.Config.AllowedOrigins` or set it in a struct literal, switch to
|
||||
> `cfg.Server.CORSOrigins`:
|
||||
>
|
||||
> ```go
|
||||
> // v1.x (removed) — compiled but could serve no CORS after v1.2.0:
|
||||
> // mw.CORS(cfg.Web.AllowedOrigins)
|
||||
> // web.New(logger, web.Config{AllowedOrigins: origins})
|
||||
>
|
||||
> // v2.0.0 — the single source of truth:
|
||||
> mw.CORS(cfg.Server.CORSOrigins) // server.New tier
|
||||
> web.New(logger, web.Config{Server: cfg.Server}) // web.New reads it automatically
|
||||
> ```
|
||||
>
|
||||
> `validate_snippet` flags any lingering `AllowedOrigins` reference (`web.allowedorigins-removed`).
|
||||
|
||||
```go
|
||||
package wire
|
||||
|
||||
|
||||
@@ -17,9 +17,52 @@ func init() {
|
||||
Module: "web",
|
||||
Check: checkCORSWildcard,
|
||||
},
|
||||
Rule{
|
||||
ID: "web.allowedorigins-removed",
|
||||
Severity: SeverityError,
|
||||
Module: "web",
|
||||
Check: checkAllowedOriginsRemoved,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// checkAllowedOriginsRemoved flags any reference to the removed
|
||||
// web.Config.AllowedOrigins field — both a selector (cfg.Web.AllowedOrigins) and a
|
||||
// struct-literal key (web.Config{AllowedOrigins: ...}). It was env-backed through
|
||||
// v1.1.x, became a code-only override in v1.2.0, and was removed in v2.0.0. Code
|
||||
// that still reads it compiled but silently served no CORS in v1.2.0; in v2.0.0 it
|
||||
// no longer compiles. CORS now lives solely on server.Config.CORSOrigins.
|
||||
func checkAllowedOriginsRemoved(c *Context) []Finding {
|
||||
const (
|
||||
msg = "web.Config.AllowedOrigins was removed in v2.0.0 — CORS lives on Server.CORSOrigins (env EINHERJAR_SERVER_CORS_ORIGINS)"
|
||||
hint = "Read cfg.Server.CORSOrigins (or set it in code); web.New applies it automatically. Never reintroduce a field/var for CORS origins."
|
||||
)
|
||||
seen := map[int]bool{}
|
||||
var hits []Finding
|
||||
add := func(pos token.Pos) {
|
||||
line := c.Fset.Position(pos).Line
|
||||
if seen[line] {
|
||||
return
|
||||
}
|
||||
seen[line] = true
|
||||
hits = append(hits, Finding{Message: msg, Hint: hint, Line: line})
|
||||
}
|
||||
ast.Inspect(c.File, func(n ast.Node) bool {
|
||||
switch e := n.(type) {
|
||||
case *ast.SelectorExpr:
|
||||
if e.Sel != nil && e.Sel.Name == "AllowedOrigins" {
|
||||
add(e.Sel.Pos())
|
||||
}
|
||||
case *ast.KeyValueExpr:
|
||||
if id, ok := e.Key.(*ast.Ident); ok && id.Name == "AllowedOrigins" {
|
||||
add(id.Pos())
|
||||
}
|
||||
}
|
||||
return true
|
||||
})
|
||||
return hits
|
||||
}
|
||||
|
||||
// checkCORSWildcard flags any call to <pkg>.CORS(...) whose arguments contain a
|
||||
// "*" string literal — which mw.CORS rejects (panics) at boot.
|
||||
func checkCORSWildcard(c *Context) []Finding {
|
||||
|
||||
@@ -38,6 +38,34 @@ func f() {
|
||||
}
|
||||
`
|
||||
|
||||
const allowedOriginsSelectorSnippet = `package wire
|
||||
|
||||
import "code.nochebuena.dev/einherjar/web/mw"
|
||||
|
||||
func f(cfg Config) { _ = mw.CORS(cfg.Web.AllowedOrigins) }
|
||||
`
|
||||
|
||||
const allowedOriginsLiteralSnippet = `package wire
|
||||
|
||||
import "code.nochebuena.dev/einherjar/web"
|
||||
|
||||
func f() { _ = web.Config{AllowedOrigins: []string{"https://x"}} }
|
||||
`
|
||||
|
||||
func TestAllowedOriginsSelectorFires(t *testing.T) {
|
||||
got := findingsFor(Run(allowedOriginsSelectorSnippet), "web.allowedorigins-removed")
|
||||
if len(got) == 0 {
|
||||
t.Fatal("web.allowedorigins-removed did not fire on cfg.Web.AllowedOrigins")
|
||||
}
|
||||
}
|
||||
|
||||
func TestAllowedOriginsLiteralFires(t *testing.T) {
|
||||
got := findingsFor(Run(allowedOriginsLiteralSnippet), "web.allowedorigins-removed")
|
||||
if len(got) == 0 {
|
||||
t.Fatal("web.allowedorigins-removed did not fire on web.Config{AllowedOrigins: ...}")
|
||||
}
|
||||
}
|
||||
|
||||
func TestCORSWildcardFires(t *testing.T) {
|
||||
got := findingsFor(Run(corsWildcardSnippet), "cors.wildcard-noop")
|
||||
if len(got) == 0 {
|
||||
|
||||
Reference in New Issue
Block a user