diff --git a/AGENTS.md b/AGENTS.md index 99214c6..2822c92 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -168,7 +168,42 @@ embedding it. with a log line while the solution served on — and it went with the registrations. Every environment read now sits in `loadConfig`'s call tree, which `environment_boundary_test.go` pins: a new read outside it fails the - suite until this file describes the exception. + suite until this file describes the exception — and so does a read reachable + from the serving surface, because a helper boot shares with serving code sits + in that tree while running long after `validate()`. +- Those gates ask the compiler, not the source text, and that is not a style + choice: four review rounds each found one more spelling a syntactic rule + lost. Constants come from `types.Info` (a conversion, a chain, a local + constant all fold); reachability is CHA over SSA from `golang.org/x/tools`, + rooted at `serve`, every exported declaration, the package initializer and + every address-taken function — a handler mounted on a mux is never *called* + here, so a graph of call expressions has no edge to it. A reader is an + OBJECT (`os.Getenv`/`LookupEnv`/`Environ`/`ExpandEnv`, `syscall`'s, the + SDK's accessors, this package's `env`) followed wherever its identity flows, + with one definition shared by both gates, and an environment key that cannot + be traced to constants is refused rather than skipped. If you add a gate + here, resolve rather than match: `facade_claim_test.go` and + `environment_boundary_test.go` carry the compiled fixtures for every spelling + that beat an earlier version. +- Resolving a value in `loadConfig` is half the rule; the other half is a + refusal in `validate()` that fires **whether or not the feature using it is + switched on**. The `api.consumes` projection was resolved at boot and decoded + only by the code that consumes it, which returns early for a solution + declaring no `Consumes` — so one undecodable projection had two answers: + refused for a solution with a passthrough, accepted and served for one + without. It is decoded now in `loadConfig` (`parseAPIConsumes`) and refused by + `validate()` either way, naming the variable it arrived in, since it is the + composition's output and no override repairs it. `projection_boundary_test.go` + pins that, including that unset and whitespace mean "consumes nothing" rather + than "malformed". +- A solution claims no module's facade route and holds no credential that would + let it: a route for a module is claimed by the module that serves it, under a + credential bound to that module (#51, SA-F-GWREGISTRY). The passthrough and + the gateway client *read* a consumed module at `/v1//*`; nothing here + decides that the prefix points there. `facade_claim_test.go` evaluates the + package's constant string expressions and fails on any that is one of those + wire shapes — folded, so a shape split across a concatenation, parenthesised, + built from named constants or cased differently is the same shape to it. - **Never sign, parse or verify a Work Context here.** There is one implementation, Core's; this runtime obtains a credential through the SDK's mint client, carries it as the string it travels as, and lets the far end diff --git a/README.md b/README.md index 696c703..b878337 100644 --- a/README.md +++ b/README.md @@ -752,6 +752,19 @@ than a log line. It used to be read at serve time: a malformed projection disabled the whole federation with one log line while the solution served on, so every consumed facade 404'd at the gateway and the solution looked healthy. +The refusal fires **whether or not the solution declares a passthrough**. Being +resolved at boot is not the same as being judged there, and for a release it +was only the first: the projection was decoded by the code that consumes it, +which returns early when there is no `Consumes` declaration, so the same +undecodable value was refused for one solution and served for another. It is +decoded once at boot now and refused either way, naming `CODEFLY__API_CONSUMES` +— the projection is the composition's output, so no override an operator can +set repairs it, and `codefly run solution` is where it is fixed. + +Unset is not malformed. A solution that consumes nothing leaves the variable +empty, and whitespace alone counts as empty, so a render emitting a bare +newline for it does not fail a boot. + ### Reading a Work-Context-authenticated module diff --git a/contract_test.go b/contract_test.go index 7dea9b3..b8fec53 100644 --- a/contract_test.go +++ b/contract_test.go @@ -21,7 +21,7 @@ func contractServer(t *testing.T, profile string, contract ModuleContract, modul modules = []ConsumedModule{passthroughModule()} } server := New(Manifest{ID: testSolutionID}).Consumes(modules...).Contract(contract) - server.cfg = config{profile: profile, apiConsumes: consumesThings} + server.cfg = config{profile: profile, consumes: newProjection(consumesThings)} server.principal = testPrincipal return server } @@ -706,7 +706,7 @@ func TestTheDeclaredCeilingIsFrozenAtDeclaration(t *testing.T) { Contract(ModuleContract{Ceilings: map[string]map[string][]Scope{ localProfile: {"things": {{ResourceKind: "things", Actions: ceilingActions}}}, }}) - server.cfg = config{gatewayURL: gw.URL, profile: localProfile, apiConsumes: consumesThings} + server.cfg = config{gatewayURL: gw.URL, profile: localProfile, consumes: newProjection(consumesThings)} server.principal = testPrincipal contract, err := server.resolveContract() if err != nil { diff --git a/environment_boundary_test.go b/environment_boundary_test.go index f1be770..d4ab3e0 100644 --- a/environment_boundary_test.go +++ b/environment_boundary_test.go @@ -4,11 +4,18 @@ import ( "go/ast" "go/parser" "go/token" + "go/types" "os" "path/filepath" "sort" "strings" "testing" + + "golang.org/x/tools/go/callgraph" + "golang.org/x/tools/go/callgraph/cha" + "golang.org/x/tools/go/packages" + "golang.org/x/tools/go/ssa" + "golang.org/x/tools/go/ssa/ssautil" ) // AGENTS.md tells an agent that configuration is resolved in one place and @@ -43,6 +50,475 @@ func TestEnvironmentIsReadOnlyWhereAgentsFileSaysItIs(t *testing.T) { } } +// And a helper that boot resolution uses must not also be used after it. +// +// The gate above asks only whether an environment reader sits in loadConfig's +// call tree. A helper can sit there and be called from serving code as well, +// and then it passes: `env` is reachable from loadConfig, so a serve-time call +// to `env` reads the environment after validate() has had its say while every +// boot-reachability check stays green. +// +// ON THE TYPE CHECKER, and over function VALUES, because the two things this +// gate must follow are exactly the two a name-matching graph loses: +// +// - `(env)(...)` is a call to env that no callee-NAME walker sees, because +// the callee is a parenthesised expression rather than an identifier; +// - a handler mounted on the mux is never "called" anywhere. `handleHealth` +// is passed to HandleFunc as a value and invoked by net/http per request, +// so a graph built from call expressions contains no edge to it at all — +// and a read inside it happens on every request, which is as far from boot +// as a read gets. +// +// So calls are resolved through types.Info, and a function mentioned as a +// VALUE is an edge too: if serving code can hand it to something, something +// can call it. Roots are `serve` plus every exported declaration, to a fixed +// point — a library's exported surface is callable by a consumer at any time, +// and a list of two names was a list an author adds to without noticing. +func TestNoEnvironmentReadIsReachableAfterBoot(t *testing.T) { + pkgs := gatePackages(t, nil) + readers := servingReaders(t, pkgs) + + var names []string + for name := range readers { + names = append(names, name) + } + sort.Strings(names) + for _, name := range names { + t.Errorf("%s is reachable from the post-boot surface without passing through loadConfig:\n %s\n"+ + "A value read after boot is never refused by validate(): it changes what a running solution does, with nothing judging it. "+ + "Resolve it in loadConfig and carry the resolved value, rather than reading the environment where it is used.", + name, readers[name]) + } +} + +// And the gate has to be able to see a read at all, or its silence means +// nothing. Every serving shape a previous version of this gate lost, in ONE +// compiled fixture: each load shells out to the go command and builds SSA for +// the whole program, so one overlay answers for all of them. Each shape has +// its own reader, and each is asserted by name, so a gate that followed one +// edge kind and missed the others fails. +func TestThePostBootGateSeesEveryServingShape(t *testing.T) { + dir, err := filepath.Abs(".") + if err != nil { + t.Fatal(err) + } + fixture := filepath.Join(dir, "zz_post_boot_fixture.go") + + source := `package solution + +import ( + "context" + "net/http" + "os" +) + +// A parenthesised callee on an exported path. +func ServedParenthesisedRead() string { return (env)("ANYTHING", "") } + +// A reader reached only as a handler value mounted on a mux, with the handler +// itself never called in this package. +func (s *Server) MountFixture(mux *http.ServeMux) { + mux.HandleFunc("/fixture", fixtureHandler) +} + +func fixtureHandler(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("x-fixture", fixtureMountedRead()) +} + +func fixtureMountedRead() string { return os.Getenv("ANYTHING") } + +// A method value stored in a package-level struct and mounted from there, so +// the only edge to it is the stored field. +type fixtureReceiver struct{} + +func (fixtureReceiver) fixtureServe(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("x-stored", fixtureStoredRead()) +} + +func fixtureStoredRead() string { return os.LookupEnvValue("ANYTHING") } + +var fixtureHandlers = struct { + h func(http.ResponseWriter, *http.Request) +}{h: fixtureReceiver{}.fixtureServe} + +func MountStoredFixture(mux *http.ServeMux) { + mux.HandleFunc("/stored", fixtureHandlers.h) +} + +// An exported variable holding a closure that reads. +var ExportedClosure = func() string { return (env)("ANYTHING", "") } + +// An ALIASED reader with a perfectly constant key, on a served path. This is +// the shape that passed both gates at once: the key resolves, so the key gate +// has nothing to say, and the call is indirect, so a CHA edge into os.Getenv +// is an over-approximation the serving graph discards. +type servingEnvReader = func(string) string + +var servingRead servingEnvReader = os.Getenv + +func servingAliasRead() string { return servingRead("ANYTHING") } + +func (s *Server) MountAliasFixture(mux *http.ServeMux) { + mux.HandleFunc("/alias", func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("x-alias", servingAliasRead()) + }) +} + +// And the whole environment at once, through a function boot also uses: +// discoverHostModule scans os.Environ, so an exported wrapper of it reads +// changing configuration after boot. os.Environ was missing from the reader +// set entirely. +func ExportedLateHost() string { + return discoverHostModule(context.Background(), "auth-gateway", "rest", "rest") +} +` + // os.LookupEnvValue does not exist; the fixture must compile, so use the + // real reader. + source = strings.ReplaceAll(source, "os.LookupEnvValue(", "os.Getenv(") + + readers := servingReaders(t, gatePackages(t, map[string][]byte{fixture: []byte(source)})) + if len(readers) == 0 { + t.Fatal("the gate reported no post-boot reader for a fixture that reads on seven serving paths") + } + // Both readers these shapes reach, so a gate that saw one kind of read and + // not the other cannot pass: env for the in-package helper, os.Getenv for + // the direct and aliased ones, os.Environ for the whole-environment scan. + for _, want := range []string{"env", "os.Getenv", "os.Environ"} { + if _, ok := readers[want]; !ok { + t.Errorf("the gate did not report %s: readers=%v", want, readers) + } + } +} + +// --- One reader definition, by object identity --- +// +// A reader is an OBJECT, not a spelling. Both gates share this, because the +// two of them disagreeing about what a read is is how a read gets past both: +// +// read := r2Reader(os.Getenv) +// read(strings.Join([]string{"CODEFLY", "", "MODULE", ...}, "_")) +// +// recognising readers by callee name sees no read at all — the callee is +// `read` — while a graph that merely records an edge to Getenv never marks it +// as a reader when its value is taken rather than called. So the identity is +// followed wherever it flows: converted, aliased, assigned, stored in a field, +// passed as an argument, returned. +// +// readerNames are the readers by package and name. os.ExpandEnv reads every +// variable named in its argument, syscall.Getenv is the same read one package +// down, and os.Environ reads all of them at once — which is why an exported +// wrapper of a function using it reads changing configuration after boot. +var readerNames = map[string]map[string]bool{ + "os": {"Getenv": true, "LookupEnv": true, "Environ": true, "ExpandEnv": true}, + "syscall": {"Getenv": true, "Environ": true}, +} + +// readerPackageOf is the import path a reader package goes by, since SSA names +// a package by path and the syntax names it by identifier. +func readerPackageOf(path string) string { return path } + +// sdkReaderNames are the SDK's own accessors, by method name. They are matched +// by name because they are methods on the SDK's query value rather than +// package functions, and the older gate's round-four finding was precisely a +// closure reading configuration through one of them. +var sdkReaderNames = map[string]bool{ + "WorkspaceConfiguration": true, "WorkspaceSecret": true, + "Environment": true, "Endpoint": true, "Secret": true, +} + +// keylessReaders read the whole environment rather than one named variable, so +// there is no key to resolve and nothing to constrain: the read itself is the +// finding when it happens after boot. +var keylessReaders = map[string]bool{"Environ": true, "ExpandEnv": true} + +// --- The serving surface, on a sound call graph --- +// +// Reachability is CHA (class hierarchy analysis) over SSA, from +// golang.org/x/tools, rather than a graph this test builds. +// +// Three rounds of hand-written graph each lost one more Go construct: a +// parenthesised callee, a handler mounted as a value, a method value stored in +// a package-level struct, a callback in a variadic option, a func field on an +// interface. Every fix was right and the next construct arrived anyway, +// because a graph assembled from the shapes somebody thought of is a graph +// about those shapes. CHA resolves an indirect call to every type-compatible +// function in the program — an over-approximation, which is the safe +// direction — so there is no "unresolved edge" category left to fail closed +// on: the analysis has an answer for every call site by construction. +// +// What this gate still chooses is the ROOTS. Everything that can run after +// boot: `serve`, which Serve calls once configuration is resolved; every +// exported function and method, this being a library a consumer calls into; +// and the package initializer, where a served callback is stored +// (`var h = struct{f func(...)}{f: method}`) and where an exported +// `var X = func() { ... }` lives. loadConfig is excluded as a NODE, so a +// helper boot shares with serving code is judged by the serving path. + +// servingReaders is every environment reader reachable from the post-boot +// roots, named by the function that reads, with the path that reaches it. +func servingReaders(t *testing.T, pkgs []*packages.Package) map[string]string { + t.Helper() + + // SSA for THIS package only, not the whole dependency closure. Building + // everything cost 40s of the suite under -race and bought nothing: the + // graph is used for in-package reachability, and re-entry into this + // package from a dependency always happens through a function whose + // address was taken — net/http invoking a mounted handler is exactly that + // — which is already a root. Reader identification does not come from + // this graph at all. + prog, _ := ssautil.Packages(pkgs, 0) + prog.Build() + graph := cha.CallGraph(prog) + graph.DeleteSyntheticNodes() + + target := "" + for _, pkg := range pkgs { + if pkg.PkgPath == "github.com/codefly-dev/solution-runtime-go" { + target = pkg.PkgPath + } + } + if target == "" { + t.Fatal("the package under test was not loaded: this gate is inert") + } + + // Address-taken functions are roots too. A handler mounted on a mux and a + // closure assigned to an exported variable are never CALLED in this + // package, so the call graph holds no edge to them — whoever holds the + // value chooses when it runs, and that is after boot. These two shapes + // survived a gate that rooted only at named entry points. + escaping := addressTaken(prog, target) + + var roots []*callgraph.Node + for fn, node := range graph.Nodes { + if fn == nil || !inPackage(fn, target) { + continue + } + if isPostBootRoot(fn) || escaping[fn] { + roots = append(roots, node) + } + } + if len(roots) == 0 { + t.Fatal("no post-boot roots found: this gate is inert") + } + + // Breadth-first from the roots, never through loadConfig. + readers := map[string]string{} + seen := map[*callgraph.Node]bool{} + path := map[*callgraph.Node]string{} + var queue []*callgraph.Node + for _, root := range roots { + if seen[root] { + continue + } + seen[root] = true + path[root] = root.Func.Name() + queue = append(queue, root) + } + for len(queue) > 0 { + current := queue[0] + queue = queue[1:] + for _, edge := range current.Out { + next := edge.Callee + if next == nil || next.Func == nil || seen[next] { + continue + } + if isBootResolution(next.Func) { + continue + } + // A reader is reported only when THIS package's code is what + // calls it. The walk still goes through dependencies, because a + // handler this package mounts is invoked by net/http and the path + // back to it runs through there — but a dependency reading its own + // configuration is not this package's boundary. Reporting every + // reader on the path named four that are all somebody else's: the + // SDK rechecking a rotated value, grpc's resolver, viper's init, + // none of which this package could resolve in loadConfig. + // Which calls are reads is the identity model's answer, below, + // and it is the same model the key gate uses: CHA resolves an + // indirect call to every type-compatible function, so a callback + // of shape func(string) (string, bool) resolves to os.LookupEnv + // whether or not anything puts it there. + if !inPackage(next.Func, target) { + continue + } + seen[next] = true + path[next] = path[current] + " → " + next.Func.Name() + queue = append(queue, next) + } + } + + // Which calls are reads comes from the reader identity model, not from + // this graph: an aliased reader — `var read reviewEnvReader = os.Getenv`, + // called through `read` — is an indirect call CHA can only guess at, while + // the identity model resolves it exactly. Each invocation is attributed to + // the innermost function containing it, and reported when that function is + // reachable after boot. + for _, pkg := range pkgs { + if pkg.PkgPath != target { + continue + } + identity := newReaderIdentity(pkg) + for _, site := range identity.invocations { + holder := innermostContaining(seen, site.pos) + if holder == nil { + continue + } + readers[site.name] = path[holder] + " → " + site.name + } + } + return readers +} + +// innermostContaining is the reachable function whose source extent contains a +// position most tightly, so a read inside a closure is attributed to the +// closure rather than to whatever encloses it. +func innermostContaining(reached map[*callgraph.Node]bool, pos token.Pos) *callgraph.Node { + var best *callgraph.Node + var bestSize token.Pos + for node := range reached { + syntax := node.Func.Syntax() + if syntax == nil { + continue + } + if pos < syntax.Pos() || pos >= syntax.End() { + continue + } + if size := syntax.End() - syntax.Pos(); best == nil || size < bestSize { + best, bestSize = node, size + } + } + return best +} + +// isPostBootRoot reports whether a function can run after boot. +func isPostBootRoot(fn *ssa.Function) bool { + if fn.Synthetic == "package initializer" || fn.Name() == "init" { + // Where a package-level callback is stored and where an exported + // func-valued variable is initialised. + return true + } + if fn.Name() == "serve" { + return true + } + // A method on an exported or unexported receiver is still reachable from a + // consumer when the method itself is exported. + return ast.IsExported(fn.Name()) +} + +// staticCall reports whether an edge is a call that names its callee, rather +// than one CHA resolved from a function type. +func staticCall(edge *callgraph.Edge, callee *ssa.Function) bool { + if edge.Site == nil { + return false + } + return edge.Site.Common().StaticCallee() == callee +} + +// addressTaken is every function of the package whose value is used as a +// value rather than called: stored in a variable or a struct, handed to a +// registration call, closed over. SSA makes this visible — a function +// appearing as an operand anywhere other than a call's static callee is one +// somebody can invoke later. +func addressTaken(prog *ssa.Program, target string) map[*ssa.Function]bool { + taken := map[*ssa.Function]bool{} + for fn := range ssautil.AllFunctions(prog) { + for _, block := range fn.Blocks { + for _, instruction := range block.Instrs { + call, isCall := instruction.(ssa.CallInstruction) + var static *ssa.Function + if isCall { + static = call.Common().StaticCallee() + } + var operands []*ssa.Value + operands = instruction.Operands(operands) + for _, operand := range operands { + if operand == nil || *operand == nil { + continue + } + referenced, ok := (*operand).(*ssa.Function) + if !ok || referenced == static { + continue + } + if referenced.Pkg != nil && referenced.Pkg.Pkg.Path() == target { + taken[referenced] = true + } + // An anonymous function's parent is what wrote it; the + // literal itself is the thing being handed out. + if referenced.Parent() != nil && referenced.Parent().Pkg != nil && + referenced.Parent().Pkg.Pkg.Path() == target { + taken[referenced] = true + } + } + } + } + } + return taken +} + +// inPackage reports whether a function belongs to the package under test. +func inPackage(fn *ssa.Function, path string) bool { + return fn.Pkg != nil && fn.Pkg.Pkg.Path() == path +} + +// isBootResolution reports whether a function IS boot configuration +// resolution, which the walk does not pass through. +func isBootResolution(fn *ssa.Function) bool { + return fn.Name() == "loadConfig" && fn.Pkg != nil && + fn.Pkg.Pkg.Path() == "github.com/codefly-dev/solution-runtime-go" +} + +// environmentReader reports whether an SSA function is one of the readers, by +// the SAME definition the key gate uses: package and name for the os and +// syscall readers, method name for the SDK's accessors, and this package's own +// env helper. +func environmentReader(fn *ssa.Function) (string, bool) { + if fn.Pkg != nil { + path := fn.Pkg.Pkg.Path() + if readerNames[path][fn.Name()] { + return path + "." + fn.Name(), true + } + if path == "github.com/codefly-dev/solution-runtime-go" && fn.Name() == "env" { + return "env", true + } + } + // The SDK's own accessors, matched by the package they belong to as well + // as the name: a bare name match reported grpc's resolver `Endpoint` as a + // configuration read, which it is not. + if fn.Pkg != nil && sdkReaderNames[fn.Name()] && strings.HasPrefix(fn.Pkg.Pkg.Path(), "github.com/codefly-dev/sdk-go") { + return "the SDK's " + fn.Name(), true + } + return "", false +} + +// unparen removes every layer of parentheses, so `(env)(...)` is a call to env +// rather than a call to something this gate has never heard of. +func unparen(expr ast.Expr) ast.Expr { + for { + paren, ok := expr.(*ast.ParenExpr) + if !ok { + return expr + } + expr = paren.X + } +} + +// isFuncTyped reports whether a type is a function type. +func isFuncTyped(t types.Type) bool { + if t == nil { + return false + } + _, ok := types.Unalias(t).Underlying().(*types.Signature) + return ok +} + +// pkgOf is the import path of the package a function belongs to. +func pkgOf(fn *types.Func) string { + if fn.Pkg() == nil { + return "" + } + return fn.Pkg().Path() +} + // And the read has to happen at boot, not merely be written there. // // Static reachability is not the claim AGENTS.md makes, and the difference was diff --git a/facade_claim_test.go b/facade_claim_test.go new file mode 100644 index 0000000..b742b4f --- /dev/null +++ b/facade_claim_test.go @@ -0,0 +1,850 @@ +package solution + +import ( + "go/ast" + "go/constant" + "go/token" + "go/types" + "path/filepath" + "sort" + "strconv" + "strings" + "testing" + + "golang.org/x/tools/go/packages" +) + +// --- A solution claims no module's facade route (SP-GW-04, SP-SOL-03) --- +// +// Two invariants: +// +// - a facade route for a module is claimed only by the module that serves +// it, under a credential bound to that module; +// - a solution holds no credential that decides where another module's +// traffic is routed. +// +// So the package carries neither a module route-claim surface nor a credential +// for one, and no reader of the carrier such a credential arrives in. +// +// ON THE TYPE CHECKER, for the reason typed_boundary_test.go gives: a rule +// that reads syntax is a rule about names, and an author picks the names. The +// first version of this gate evaluated constants by hand — literals, +// parentheses, `+`, and names bound to those — and skipped what it could not +// evaluate. A skip is a silent pass, and this compiles to the forbidden key +// while matching none of its fragments: +// +// const key = "CODEFLY__MODULE_" + string(rune(82)) + "EGISTRATION_SECRETS" +// +// `string(rune(82))` is a constant conversion, which the hand evaluator did not +// implement. There is no list of expression forms to finish here: the compiler +// has already folded every constant, conversions and concatenation alike, so +// the value is taken from it. That fixture is a compiled negative case below. +var claimShapes = map[string]string{ + "/modules/_register": "the gateway's module route-claim path", + "/modules/_registration-token": "the exchange that credentials a module route claim", + "X-Codefly-Module-Secret": "the header carrying a module's route-claim secret", + "X-Codefly-Module-Registration": "the header carrying a module route-claim credential", + "CODEFLY__MODULE_REGISTRATION_SECRETS": "the carrier delivering another module's route-claim secret", +} + +// TestNoModuleFacadeClaimOnTheSDKSurface refuses any constant string in the +// package whose folded value is one of those shapes. +func TestNoModuleFacadeClaimOnTheSDKSurface(t *testing.T) { + for _, pkg := range gatePackages(t, nil) { + if findings := claimShapeFindings(t, pkg); len(findings) > 0 { + for _, finding := range findings { + t.Errorf("%s\na solution claims no module's facade route and holds no credential that could: a route for a module is claimed by the module that serves it, under a credential bound to that module", + finding) + } + } + } +} + +// claimShapeFindings is every constant expression in pkg whose folded value is +// a claim shape. It is the rule itself, so the negative fixtures below drive +// exactly what the gate drives. +func claimShapeFindings(t *testing.T, pkg *packages.Package) []string { + t.Helper() + + var findings []string + evaluated := 0 + for _, file := range pkg.Syntax { + name := filepath.Base(pkg.Fset.Position(file.Pos()).Filename) + // Test sources are exempt: the shapes are written down here on + // purpose, which is what lets this assert their absence elsewhere. + if strings.HasSuffix(name, "_test.go") { + continue + } + ast.Inspect(file, func(n ast.Node) bool { + expr, ok := n.(ast.Expr) + if !ok { + return true + } + value, ok := foldedString(pkg, expr) + if !ok { + return true + } + evaluated++ + for shape, what := range claimShapes { + if strings.Contains(foldShape(value), foldShape(shape)) { + findings = append(findings, pkg.Fset.Position(expr.Pos()).String()+ + ": the constant "+strconv.Quote(value)+" is "+what+" ("+shape+")") + } + } + // The whole expression folded, so its parts add only noise. + return false + }) + } + if evaluated == 0 { + t.Fatalf("%s: no constant strings evaluated, so this gate is inert", pkg.PkgPath) + } + sort.Strings(findings) + return findings +} + +// foldedString is the constant string an expression evaluates to, as the +// COMPILER folded it. Nothing is reimplemented: `string(rune(82)) + "x"` and a name +// bound to it arrive here already folded, which is the whole reason the hand +// evaluator is gone. +func foldedString(pkg *packages.Package, expr ast.Expr) (string, bool) { + tv, ok := pkg.TypesInfo.Types[expr] + if !ok || tv.Value == nil || tv.Value.Kind() != constant.String { + return "", false + } + return constant.StringVal(tv.Value), true +} + +// foldShape removes what spelling may change without changing the wire shape: +// case, and the separators these paths, headers and carriers are punctuated +// with. +func foldShape(s string) string { + var b strings.Builder + for _, r := range strings.ToLower(s) { + switch r { + case '-', '_', '/', '.', ' ', '\t': + default: + b.WriteRune(r) + } + } + return b.String() +} + +// --- An environment read is refused unless its key is a resolved constant --- + +// TestEveryEnvironmentKeyIsAResolvedConstant refuses an environment read +// whose key cannot be traced to constants. +// +// BY READER IDENTITY, shared with the post-boot gate, because recognising a +// reader by the callee's name is how this escaped: +// +// read := r2Reader(os.Getenv) +// read(strings.Join([]string{"CODEFLY", "", "MODULE", ...}, "_")) +// +// The callee is `read`, so a name rule sees no read at all, and the key — the +// forbidden carrier, assembled at run time — is never examined. The reader is +// an object now, followed wherever its identity flows: converted, aliased, +// assigned, stored in a field, passed as an argument. +// +// An unresolved key is a REFUSAL rather than a skip, which is the difference +// between a gate and a suggestion. Traced, not demanded in place: this package +// names its keys as exported constants and passes them through a parameter or +// a struct field, which is good code and stays possible, so a key that does +// not fold is followed to its sources and anything bottoming out in a computed +// value is refused. +func TestEveryEnvironmentKeyIsAResolvedConstant(t *testing.T) { + for _, pkg := range gatePackages(t, nil) { + if pkg.PkgPath != "github.com/codefly-dev/solution-runtime-go" { + continue + } + for _, finding := range unresolvedKeyFindings(t, pkg) { + t.Errorf("%s\nan environment key that cannot be traced to constants cannot be judged by any static gate, so it is refused here rather than skipped: name the key as a constant, or pass one in", + finding) + } + } +} + +// unresolvedKeyFindings is every reader invocation in pkg whose key does not +// resolve. The reader set comes from the call graph, which seeds it with the +// reader objects and grows it through every flow that can carry one. +func unresolvedKeyFindings(t *testing.T, pkg *packages.Package) []string { + t.Helper() + identity := newReaderIdentity(pkg) + if len(identity.readerAliases) == 0 { + t.Fatal("no readers identified at all: this gate is inert") + } + return identity.unresolvedKeys +} + +// --- The fixture that escaped, as a compiled negative case --- + +// TestTheFacadeGatesCatchTheEscapingFixture drives both gates against the +// sources that defeated the hand evaluator, compiled into this very package. +// +// A fixture that only parsed would prove nothing — it is the compiler's +// folding of `string(rune(82))` and of a constant chain that makes these keys +// real, and a parse-only fixture folds nothing. gatePackages fails the load if +// it does not type-check, so this cannot quietly become a fixture about +// nothing. +// +// All of them in ONE overlay, because each load shells out to the go command: +// the shapes are independent declarations, and the rules return every finding, +// so one compile answers for all of them. +func TestTheFacadeGatesCatchTheEscapingFixture(t *testing.T) { + dir, err := filepath.Abs(".") + if err != nil { + t.Fatal(err) + } + fixture := filepath.Join(dir, "zz_facade_claim_fixture.go") + + source := `package solution + +import "os" + +// A constant conversion splicing the carrier's name. +func FacadeCredentialViaConversion() string { + const key = "CODEFLY__MODULE_" + string(rune(82)) + "EGISTRATION_SECRETS" + return env(key, "") +} + +// A claim path spliced the same way. +const facadeClaimPath = "/modules/" + string(rune(95)) + "register" + +func FacadeClaimPath() string { return facadeClaimPath } + +// A header assembled through a conversion. +func FacadeSecretHeader() string { + return "X-Codefly-Module-" + string(rune(83)) + "ecret" +} + +// Function-local constants, which a package-level-only collector never saw. +func FacadeClaimLocal(gatewayURL string) string { + const claim = "/modules/" + "_register" + return gatewayURL + claim +} + +func FacadeCredentialLocal() string { + const carrier = "CODEFLY__MODULE" + "_REGISTRATION_SECRETS" + return env(carrier, "") +} + +// A chain longer than any iteration limit, declared in reverse so a single +// forward pass resolves none of it. +const ( + chain00 = "/" + chain01 + chain01 = "m" + chain02 + chain02 = "o" + chain03 + chain03 = "d" + chain04 + chain04 = "u" + chain05 + chain05 = "l" + chain06 + chain06 = "e" + chain07 + chain07 = "s" + chain08 + chain08 = "/" + chain09 + chain09 = "_" + chain10 + chain10 = "r" + chain11 + chain11 = "e" + chain12 + chain12 = "g" + chain13 + chain13 = "i" + chain14 + chain14 = "s" + chain15 + chain15 = "t" + chain16 + chain16 = "e" + chain17 + chain17 = "r" +) + +func FacadeClaimChain() string { return chain00 } + +// And a key that is not constant at all. +func FacadeCredentialViaRuntimeKey(parts []string) string { + return env(joinedKey(parts), "") +} + +// The reader's identity hidden behind a local function type: the callee is +// ` + "`read`" + `, so no rule about callee names sees a read at all, and the key is +// never examined. +type aliasedReader = func(string) string + +func AliasedCredential(parts []string) string { + read := aliasedReader(osGetenv) + return read(joinedKey(parts)) +} + +// Through a struct field, through a parameter, and parenthesised. +type readerBox struct{ read func(string) string } + +func AliasedThroughAField(parts []string) string { + box := readerBox{read: osGetenv} + return box.read(joinedKey(parts)) +} + +func aliasedThroughAParameter(read func(string) string, parts []string) string { + return read(joinedKey(parts)) +} + +func AliasedCaller(parts []string) string { + return aliasedThroughAParameter(osGetenv, parts) +} + +func AliasedThroughParentheses(parts []string) string { + return (env)(joinedKey(parts), "") +} + +var osGetenv = os.Getenv + +func joinedKey(parts []string) string { + out := "" + for _, part := range parts { + out += part + } + return out +} +` + + // One load for both rules: each shells out to the go command, and the + // fixture is the same package either way. + shapes, keys := "", "" + for _, pkg := range gatePackages(t, map[string][]byte{fixture: []byte(source)}) { + if pkg.PkgPath != "github.com/codefly-dev/solution-runtime-go" { + continue + } + shapes = strings.Join(claimShapeFindings(t, pkg), "\n") + keys = strings.Join(unresolvedKeyFindings(t, pkg), "\n") + } + for _, want := range []string{ + "CODEFLY__MODULE_REGISTRATION_SECRETS", + "/modules/_register", + "X-Codefly-Module-Secret", + } { + if !strings.Contains(shapes, want) { + t.Errorf("the claim-shape gate did not catch %q.\nfindings:\n%s", want, shapes) + } + } + // Every spelling, not just one of each shape: the chain and the local + // constant both produce the claim path, so two findings name it. + if strings.Count(shapes, "/modules/_register") < 2 { + t.Errorf("the claim path was caught in only one spelling.\nfindings:\n%s", shapes) + } + + // Four aliased reader invocations: through a conversion, a struct field, + // a parameter, and parenthesised — plus the runtime-joined key. Each is a + // spelling that defeated recognising readers by callee name. + if got := strings.Count(keys, "does not resolve to constants"); got < 5 { + t.Errorf("the key gate caught %d of the five untraceable-key invocations.\nfindings:\n%s", got, keys) + } +} + +// And the gates must not fire on the package as it is, or the fixtures above +// prove nothing: a gate that fires on everything catches every fixture. +func TestTheFacadeGatesAreSilentOnThisPackage(t *testing.T) { + for _, pkg := range gatePackages(t, nil) { + if pkg.PkgPath != "github.com/codefly-dev/solution-runtime-go" { + continue + } + if findings := claimShapeFindings(t, pkg); len(findings) > 0 { + t.Errorf("the claim-shape gate fires on the package as it is: %s", strings.Join(findings, "; ")) + } + if findings := unresolvedKeyFindings(t, pkg); len(findings) > 0 { + t.Errorf("the key gate fires on the package as it is: %s", strings.Join(findings, "; ")) + } + } +} + +// --- Reader identity, shared with the post-boot gate --- + +// readerIdentity is the set of objects that may hold an environment reader, +// and the reader invocations whose key does not resolve to constants. +// +// A reader is an OBJECT, not a spelling. The gates agreeing about what a read +// is matters because two definitions disagreeing is how a read gets past both: +// a reader converted to a local function type and called through that name is +// invisible to a rule that matches callee names, and its key is then never +// examined at all. +type readerIdentity struct { + pkg *packages.Package + // readerAliases is seeded with the reader objects and grown through every + // flow that can carry one, to a fixpoint. + readerAliases map[types.Object]bool + // unresolvedKeys is the findings: a resolved reader invocation whose key + // is not traceable to constants. + unresolvedKeys []string + // invocations is every resolved reader invocation, wherever it is. The + // post-boot gate uses these rather than its own notion of a reader call: + // one model, so an alias the key gate resolves is an alias that gate sees + // too. + invocations []readerSite +} + +// readerSite is one resolved reader invocation. +type readerSite struct { + pos token.Pos + name string +} + +func newReaderIdentity(pkg *packages.Package) *readerIdentity { + r := &readerIdentity{pkg: pkg, readerAliases: map[types.Object]bool{}} + r.seed() + for round := 0; round < 16; round++ { + if !r.grow() { + break + } + } + r.checkInvocations() + return r +} + +// files is this package's non-test sources. +func (r *readerIdentity) files() []*ast.File { + var out []*ast.File + for _, file := range r.pkg.Syntax { + if strings.HasSuffix(filepath.Base(r.pkg.Fset.Position(file.Pos()).Filename), "_test.go") { + continue + } + out = append(out, file) + } + return out +} + +// seed puts the reader objects themselves into the set, by the same definition +// the post-boot gate uses: the os and syscall readers, and this package's env. +func (r *readerIdentity) seed() { + for _, file := range r.files() { + ast.Inspect(file, func(n ast.Node) bool { + ident, ok := n.(*ast.Ident) + if !ok { + return true + } + fn, isFunc := r.pkg.TypesInfo.Uses[ident].(*types.Func) + if !isFunc || fn.Pkg() == nil { + return true + } + if readerNames[fn.Pkg().Path()][fn.Name()] { + r.readerAliases[fn] = true + } + if fn.Pkg() == r.pkg.Types && fn.Name() == "env" { + r.readerAliases[fn] = true + } + return true + }) + } +} + +// grow adds one round of objects that may hold a reader. +func (r *readerIdentity) grow() bool { + learned := false + add := func(obj types.Object) { + if obj != nil && !r.readerAliases[obj] { + r.readerAliases[obj] = true + learned = true + } + } + for _, file := range r.files() { + ast.Inspect(file, func(n ast.Node) bool { + switch node := n.(type) { + case *ast.AssignStmt: + for i, rhs := range node.Rhs { + if i < len(node.Lhs) && r.holds(rhs) { + add(r.object(node.Lhs[i])) + } + } + case *ast.ValueSpec: + for i, value := range node.Values { + if i < len(node.Names) && r.holds(value) { + add(r.pkg.TypesInfo.Defs[node.Names[i]]) + } + } + case *ast.CompositeLit: + for _, element := range node.Elts { + kv, ok := element.(*ast.KeyValueExpr) + if !ok || !r.holds(kv.Value) { + continue + } + if key, ok := kv.Key.(*ast.Ident); ok { + add(r.field(node, key.Name)) + } + } + case *ast.CallExpr: + r.intoParameters(node, add) + } + return true + }) + } + return learned +} + +// intoParameters adds a callee's parameters that receive a reader. +func (r *readerIdentity) intoParameters(call *ast.CallExpr, add func(types.Object)) { + fn, ok := r.object(call.Fun).(*types.Func) + if !ok || fn.Pkg() != r.pkg.Types { + return + } + decl := r.decl(fn) + if decl == nil || decl.Type.Params == nil { + return + } + index := 0 + for _, field := range decl.Type.Params.List { + for _, name := range field.Names { + if index < len(call.Args) && r.holds(call.Args[index]) { + add(r.pkg.TypesInfo.Defs[name]) + } + index++ + } + } +} + +// holds reports whether an expression evaluates to a reader: the object, a +// parenthesisation, a CONVERSION of one — which is the same function wearing a +// different type name — or an object already known to hold one. +func (r *readerIdentity) holds(expr ast.Expr) bool { + switch e := expr.(type) { + case *ast.ParenExpr: + return r.holds(e.X) + case *ast.Ident, *ast.SelectorExpr: + return r.readerAliases[r.object(expr)] + case *ast.CallExpr: + if tv, ok := r.pkg.TypesInfo.Types[e.Fun]; ok && tv.IsType() && len(e.Args) == 1 { + return r.holds(e.Args[0]) + } + } + return false +} + +// checkInvocations requires every resolved reader invocation's key to be +// traceable to constants. A keyless reader reads the whole environment, so +// there is no key: that read is the post-boot gate's finding, not this one's. +func (r *readerIdentity) checkInvocations() { + tracer := newKeyTracer(r.pkg) + for _, file := range r.files() { + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + if tv, ok := r.pkg.TypesInfo.Types[call.Fun]; ok && tv.IsType() { + return true // a conversion, not a call + } + callee := r.object(call.Fun) + if callee == nil || !r.readerAliases[callee] { + return true + } + r.invocations = append(r.invocations, readerSite{pos: call.Pos(), name: readerDisplayName(callee)}) + if fn, ok := callee.(*types.Func); ok && keylessReaders[fn.Name()] { + return true + } + if len(call.Args) == 0 || tracer.insideEnvHelper(call) { + return true + } + if why, ok := tracer.resolves(call.Args[0], 0); !ok { + r.unresolvedKeys = append(r.unresolvedKeys, + r.pkg.Fset.Position(call.Pos()).String()+": the key of this environment read does not resolve to constants ("+why+")") + } + return true + }) + } + sort.Strings(r.unresolvedKeys) +} + +// readerDisplayName names a reader for a finding. +func readerDisplayName(obj types.Object) string { + if fn, ok := obj.(*types.Func); ok && fn.Pkg() != nil { + if fn.Pkg().Path() == "github.com/codefly-dev/solution-runtime-go" { + return fn.Name() + } + return fn.Pkg().Name() + "." + fn.Name() + } + if obj != nil { + return obj.Name() + } + return "an environment reader" +} + +// object resolves an expression to the object it names. +func (r *readerIdentity) object(expr ast.Expr) types.Object { + switch e := unparen(expr).(type) { + case *ast.Ident: + if obj := r.pkg.TypesInfo.Uses[e]; obj != nil { + return obj + } + return r.pkg.TypesInfo.Defs[e] + case *ast.SelectorExpr: + if selection, ok := r.pkg.TypesInfo.Selections[e]; ok { + return selection.Obj() + } + return r.pkg.TypesInfo.Uses[e.Sel] + } + return nil +} + +// field is the struct field a composite literal key names. +func (r *readerIdentity) field(lit *ast.CompositeLit, name string) types.Object { + structType, ok := structTypeOf(r.pkg.TypesInfo.TypeOf(lit)) + if !ok { + return nil + } + for i := 0; i < structType.NumFields(); i++ { + if structType.Field(i).Name() == name { + return structType.Field(i) + } + } + return nil +} + +// decl is the declaration of one of this package's functions. +func (r *readerIdentity) decl(fn *types.Func) *ast.FuncDecl { + for _, file := range r.files() { + for _, d := range file.Decls { + if decl, ok := d.(*ast.FuncDecl); ok && decl.Name != nil && + r.pkg.TypesInfo.Defs[decl.Name] == fn { + return decl + } + } + } + return nil +} + +// keyTracer follows an environment key back to the constants that reach it. +type keyTracer struct { + pkg *packages.Package + // decls is every function in the package by its object, so a parameter can + // be followed to the call sites that supply it. + decls map[types.Object]*ast.FuncDecl + // envHelper is the package's own env helper, whose body is exempt. + envHelper *ast.FuncDecl +} + +func newKeyTracer(pkg *packages.Package) *keyTracer { + tracer := &keyTracer{pkg: pkg, decls: map[types.Object]*ast.FuncDecl{}} + for _, file := range pkg.Syntax { + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Name == nil { + continue + } + if obj := pkg.TypesInfo.Defs[fn.Name]; obj != nil { + tracer.decls[obj] = fn + } + if fn.Name.Name == "env" && fn.Recv == nil { + tracer.envHelper = fn + } + } + } + return tracer +} + +// insideEnvHelper reports whether a call sits in the env helper's own body. +func (k *keyTracer) insideEnvHelper(call *ast.CallExpr) bool { + if k.envHelper == nil || k.envHelper.Body == nil { + return false + } + return call.Pos() >= k.envHelper.Body.Pos() && call.End() <= k.envHelper.Body.End() +} + +// maxKeyTraceDepth bounds the walk. Nothing here needs more than a couple of +// hops, and a bound is what keeps a cycle from hanging the suite. +const maxKeyTraceDepth = 8 + +// resolves reports whether every value that can reach expr is a constant, +// and if not, what stopped the trace. +func (k *keyTracer) resolves(expr ast.Expr, depth int) (string, bool) { + if depth > maxKeyTraceDepth { + return "the trace is deeper than " + strconv.Itoa(maxKeyTraceDepth) + " hops", false + } + if _, folded := foldedString(k.pkg, expr); folded { + return "", true + } + switch e := expr.(type) { + case *ast.ParenExpr: + return k.resolves(e.X, depth+1) + case *ast.Ident: + return k.resolvesIdent(e, depth) + case *ast.SelectorExpr: + return k.resolvesField(e, depth) + } + return "it is computed here rather than named", false +} + +// resolvesIdent follows a name: a constant resolves, and a parameter resolves +// when every call site supplies something that resolves. +func (k *keyTracer) resolvesIdent(ident *ast.Ident, depth int) (string, bool) { + obj := k.pkg.TypesInfo.Uses[ident] + if obj == nil { + return "its declaration could not be resolved", false + } + if _, isConst := obj.(*types.Const); isConst { + return "", true + } + fn, index, ok := k.parameterOf(obj) + if !ok { + return "it is a variable, not a constant or a parameter carrying one", false + } + sites := k.callSites(fn) + if len(sites) == 0 { + // A parameter nothing supplies cannot be judged, and an unjudged key + // is the thing this gate exists to refuse. + return "it is a parameter of " + fn.Name.Name + ", which nothing in this package calls", false + } + for _, site := range sites { + if index >= len(site.Args) { + return "a call to " + fn.Name.Name + " passes too few arguments to judge", false + } + if why, ok := k.resolves(site.Args[index], depth+1); !ok { + return "it is a parameter of " + fn.Name.Name + ", and a caller supplies a value that " + why, false + } + } + return "", true +} + +// resolvesField follows a field read, for the shape the admission table uses: +// ranging over a composite literal of constants. +func (k *keyTracer) resolvesField(sel *ast.SelectorExpr, depth int) (string, bool) { + base, ok := sel.X.(*ast.Ident) + if !ok { + return "it is a field of something this gate cannot follow", false + } + obj := k.pkg.TypesInfo.Uses[base] + if obj == nil { + if def := k.pkg.TypesInfo.Defs[base]; def != nil { + obj = def + } + } + if obj == nil { + return "the value it is a field of could not be resolved", false + } + literals := k.rangedLiterals(obj) + if len(literals) == 0 { + return "it is a field of a value this gate cannot follow to a literal", false + } + for _, lit := range literals { + for _, element := range lit.Elts { + item, ok := element.(*ast.CompositeLit) + if !ok { + return "an element of the ranged literal is not itself a literal", false + } + value, ok := k.fieldValue(item, sel.Sel.Name) + if !ok { + return "an element of the ranged literal does not set " + sel.Sel.Name, false + } + if why, ok := k.resolves(value, depth+1); !ok { + return "a literal element sets " + sel.Sel.Name + " to a value that " + why, false + } + } + } + return "", true +} + +// parameterOf reports the function a parameter belongs to and its position. +func (k *keyTracer) parameterOf(obj types.Object) (*ast.FuncDecl, int, bool) { + for fnObj, fn := range k.decls { + signature, ok := fnObj.Type().(*types.Signature) + if !ok { + continue + } + params := signature.Params() + for i := 0; i < params.Len(); i++ { + if params.At(i) == obj { + return fn, i, true + } + } + } + return nil, 0, false +} + +// callSites is every call to fn in this package's non-test sources. +func (k *keyTracer) callSites(fn *ast.FuncDecl) []*ast.CallExpr { + target := k.pkg.TypesInfo.Defs[fn.Name] + var sites []*ast.CallExpr + for _, file := range k.pkg.Syntax { + if strings.HasSuffix(filepath.Base(k.pkg.Fset.Position(file.Pos()).Filename), "_test.go") { + continue + } + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + var name *ast.Ident + switch callee := call.Fun.(type) { + case *ast.Ident: + name = callee + case *ast.SelectorExpr: + name = callee.Sel + default: + return true + } + if k.pkg.TypesInfo.Uses[name] == target { + sites = append(sites, call) + } + return true + }) + } + return sites +} + +// rangedLiterals is every composite literal that obj ranges over. +func (k *keyTracer) rangedLiterals(obj types.Object) []*ast.CompositeLit { + var literals []*ast.CompositeLit + for _, file := range k.pkg.Syntax { + ast.Inspect(file, func(n ast.Node) bool { + stmt, ok := n.(*ast.RangeStmt) + if !ok { + return true + } + value, ok := stmt.Value.(*ast.Ident) + if !ok { + return true + } + if k.pkg.TypesInfo.Defs[value] != obj && k.pkg.TypesInfo.Uses[value] != obj { + return true + } + if lit, ok := stmt.X.(*ast.CompositeLit); ok { + literals = append(literals, lit) + } + return true + }) + } + return literals +} + +// fieldValue is the value a composite literal gives one named field, keyed or +// positional. +// +// Positional is resolved through the type checker rather than the literal's +// own syntax: the admission table is a slice of an anonymous struct, so its +// elements carry no type of their own to count fields in. +func (k *keyTracer) fieldValue(lit *ast.CompositeLit, field string) (ast.Expr, bool) { + for _, element := range lit.Elts { + if kv, ok := element.(*ast.KeyValueExpr); ok { + if key, ok := kv.Key.(*ast.Ident); ok && key.Name == field { + return kv.Value, true + } + } + } + if len(lit.Elts) == 0 { + return nil, false + } + if _, keyed := lit.Elts[0].(*ast.KeyValueExpr); keyed { + return nil, false + } + structType, ok := structTypeOf(k.pkg.TypesInfo.TypeOf(lit)) + if !ok { + return nil, false + } + for i := 0; i < structType.NumFields() && i < len(lit.Elts); i++ { + if structType.Field(i).Name() == field { + return lit.Elts[i], true + } + } + return nil, false +} + +// structTypeOf unwraps a type to the struct underneath it, through pointers +// and named types alike. +func structTypeOf(t types.Type) (*types.Struct, bool) { + if t == nil { + return nil, false + } + t = types.Unalias(t) + if pointer, ok := t.(*types.Pointer); ok { + t = types.Unalias(pointer.Elem()) + } + structType, ok := t.Underlying().(*types.Struct) + return structType, ok +} diff --git a/passthrough.go b/passthrough.go index 354d8a1..2934e68 100644 --- a/passthrough.go +++ b/passthrough.go @@ -299,11 +299,13 @@ func (s *Server) validatePassthrough() (map[string]passthroughRoute, error) { if err != nil || len(s.consumed) == 0 { return routes, err } - consumed, err := manifest.ParseConsumedAPIs(s.cfg.apiConsumes) - if err != nil { - return nil, fmt.Errorf("consumed modules cannot be checked against api.consumes: %w", err) + // Decoded at boot (loadConfig), where validate() has already refused an + // undecodable one by name. Reported here rather than assumed away: the + // seam below builds a config without going through validate(). + if s.cfg.consumes.err != nil { + return nil, fmt.Errorf("consumed modules cannot be checked against api.consumes: %w", s.cfg.consumes.err) } - return routes, checkConsumed(s.consumed, consumed) + return routes, checkConsumed(s.consumed, s.cfg.consumes.targets) } // mountPassthrough mounts the declared routes on mux at PassthroughPathPrefix: @@ -362,7 +364,10 @@ func passthroughSeam(server any, gatewayURL, consumesJSON string) (http.Handler, if len(s.consumed) == 0 { return nil, fmt.Errorf("solution %q declares no consumed modules (Consumes)", s.manifest.ID) } - s.cfg.gatewayURL, s.cfg.apiConsumes = gatewayURL, consumesJSON + // The seam supplies the projection instead of resolving it, so it decodes + // here what loadConfig decodes at a boot — through the same constructor, + // which is the only one there is. + s.cfg.gatewayURL, s.cfg.consumes = gatewayURL, newProjection(consumesJSON) routes, err := s.validatePassthrough() if err != nil { return nil, fmt.Errorf("solution %q: %w", s.manifest.ID, err) diff --git a/projection_boundary_test.go b/projection_boundary_test.go new file mode 100644 index 0000000..f67bbe3 --- /dev/null +++ b/projection_boundary_test.go @@ -0,0 +1,287 @@ +package solution + +import ( + "context" + "strings" + "testing" + + "github.com/codefly-dev/core/solution/manifest" +) + +// --- The api.consumes projection is judged at boot, whatever is declared --- +// +// The projection is resolved in loadConfig, and used to be decoded only by the +// code that consumes it. That code returns early for a solution declaring no +// passthrough, so one undecodable value had two answers: refused for a +// solution with a declaration, accepted and served for one without. Resolving +// a value at boot is not the same as judging it, and this is where the second +// half is pinned. + +// projectionOf is the projection a composition emits for one consumed target. +func projectionOf(as string) string { + return `[{"id":"things.` + as + `","module":"things","service":"` + as + + `","endpoint":"rest","protocol":"rest","as":"` + as + `"}]` +} + +// bootableEnvironment sets what loadConfig needs for validate() to get as far +// as the projection, so a test that changes the projection sees validate() +// judge the projection rather than the first unresolved value beside it. +func bootableEnvironment(t *testing.T) { + t.Helper() + t.Setenv("PORT", "8090") + t.Setenv("GATEWAY_URL", "https://gateway.cell.test:42152") + t.Setenv(CredentialMintURLEnvironmentVariable, "https://mint.cell.test/mint") + t.Setenv(ContractProfileEnvironmentVariable, "test") + t.Setenv("CODEFLY__RUNTIME_CONTEXT", "") +} + +// bootConfig is the config a boot resolves in that environment, asserted to be +// otherwise valid so each case below turns on its projection alone. +func bootConfig(t *testing.T, projection string) config { + t.Helper() + bootableEnvironment(t) + t.Setenv(manifest.APIConsumesEnvironmentVariable, projection) + return loadConfig(context.Background(), "solution-under-test", nil) +} + +// TestProjectionIsRefusedBeforeServingWhateverIsDeclared: a projection that +// will not decode fails the boot, with no passthrough declared — the case that +// used to reach no check at all. +func TestProjectionIsRefusedBeforeServingWhateverIsDeclared(t *testing.T) { + // A control first: the same environment with a good projection must boot, + // or every refusal below could be about something else entirely. + if err := bootConfig(t, projectionOf("things")).validate(); err != nil { + t.Fatalf("the test environment does not boot with a usable projection, so no refusal below can be attributed to the projection: %v", err) + } + + for _, tc := range []struct { + name string + raw string + }{ + {"truncated json", `[{"id":"a","module":"m"`}, + {"an object where a list belongs", `{"as":"things"}`}, + {"a scalar", `5`}, + {"a field of the wrong type", `[{"as":5}]`}, + {"not json at all", `nope`}, + {"a trailing comma", `[{"as":"things"},]`}, + {"two documents", `[] []`}, + } { + t.Run(tc.name, func(t *testing.T) { + err := bootConfig(t, tc.raw).validate() + if err == nil { + t.Fatalf("validate() accepted the projection %q: a value nothing decodes is a value nothing refuses", tc.raw) + } + // And through the real entry point, not only the validator. The + // refusal is only a boot refusal if the boot path runs it: with + // `start` no longer calling validate() at all, every assertion + // above still passed, because they call it themselves. + if err := startRefusal(t, tc.raw); err == nil { + t.Errorf("start() accepted the projection %q: validate() refusing it is not a boot refusal unless the boot path asks", tc.raw) + } else if !strings.Contains(err.Error(), manifest.APIConsumesEnvironmentVariable) { + t.Errorf("start() refused for some other reason than the projection: %v", err) + } + // The refusal has to name what to go and fix. The projection is + // the composition's output, not an operator's setting, so the + // variable carrying it is the only handle there is. + if !strings.Contains(err.Error(), manifest.APIConsumesEnvironmentVariable) { + t.Errorf("refusal does not name %s: %v", manifest.APIConsumesEnvironmentVariable, err) + } + }) + } +} + +// And the inverse. A solution that consumes nothing is the common case and +// must be unaffected, and whitespace means "nothing" rather than "malformed" — +// refusing a render's bare newline would fail a boot over nothing. +func TestAUsableOrAbsentProjectionIsNotRefused(t *testing.T) { + for _, tc := range []struct { + name string + raw string + }{ + {"unset", ""}, + {"whitespace only", " \n "}, + {"an empty list", `[]`}, + {"json null", `null`}, + {"one target", projectionOf("things")}, + {"several targets", `[{"id":"a.b","module":"a","service":"b","endpoint":"rest","protocol":"rest","as":"b"},` + + `{"id":"c.d","module":"c","service":"d","endpoint":"rest","protocol":"rest","as":"d"}]`}, + } { + t.Run(tc.name, func(t *testing.T) { + cfg := bootConfig(t, tc.raw) + if cfg.consumes.err != nil { + t.Fatalf("projection %q did not decode: %v", tc.raw, cfg.consumes.err) + } + if err := cfg.validate(); err != nil { + t.Fatalf("validate() refused the usable projection %q: %v", tc.raw, err) + } + }) + } +} + +// TestADeclaredPassthroughIsCheckedAgainstTheProjection proves the agreement +// check itself, on a declaration that is otherwise valid. +// +// The earlier version of this test declared a module with no methods, which +// resolvePassthrough refuses before any projection is consulted — and that +// refusal happened to contain the alias the assertion looked for, so the test +// passed without the agreement check running at all. Removing the check left +// it green. Both directions are asserted here, from one declaration that +// differs only in whether the projection lists it. +func TestADeclaredPassthroughIsCheckedAgainstTheProjection(t *testing.T) { + declared := passthroughModule() // valid: an as, scopes, and a resolvable method + + t.Run("listed in the projection", func(t *testing.T) { + s := New(Manifest{ID: "solution-under-test", Title: "Solution Under Test"}) + s.consumed = []ConsumedModule{declared} + s.cfg = bootConfig(t, projectionOf(declared.As)) + if _, err := s.validatePassthrough(); err != nil { + t.Fatalf("a declared module the projection lists was refused: %v", err) + } + }) + + t.Run("absent from the projection", func(t *testing.T) { + s := New(Manifest{ID: "solution-under-test", Title: "Solution Under Test"}) + s.consumed = []ConsumedModule{declared} + s.cfg = bootConfig(t, projectionOf("something-else")) + _, err := s.validatePassthrough() + if err == nil { + t.Fatal("a declared module absent from the projection was accepted") + } + // Specifically the agreement refusal, not resolvePassthrough's: the + // declaration above is valid, so nothing earlier can refuse it, and + // the message has to be the one about api.consumes. + if !strings.Contains(err.Error(), "api.consumes") || !strings.Contains(err.Error(), declared.As) { + t.Errorf("refusal is not the projection-agreement one: %v", err) + } + }) +} + +// TestTheSeamJudgesItsOwnProjection: the test seam supplies the projection +// instead of resolving it, so it decodes there what loadConfig decodes at a +// boot — otherwise a consumer's test would accept what a boot refuses. +// +// The refusal must be the DECODE refusal. Asserting only that an error came +// back proved nothing: with the seam's decode removed, the projection is +// simply empty, the declared module is then absent from it, and the agreement +// check refuses — a different refusal, for a different reason, that this test +// happily counted as success. A mutation that deleted the decode left it +// green, which is the defect it now fails on. +func TestTheSeamJudgesItsOwnProjection(t *testing.T) { + s := New(Manifest{ID: "solution-under-test", Title: "Solution Under Test"}) + s.consumed = []ConsumedModule{passthroughModule()} + _, err := passthroughSeam(s, "https://gateway.cell.test", `{"not":"a list"}`) + if err == nil { + t.Fatal("the seam accepted a projection a boot would refuse") + } + if !strings.Contains(err.Error(), "cannot be checked against api.consumes") { + t.Errorf("the seam refused for some other reason than the projection failing to decode: %v", err) + } +} + +// And the seam accepts the projection it should: the same declaration, listed. +// Without this, the test above is satisfied by a seam that refuses everything. +func TestTheSeamAcceptsAProjectionThatListsTheDeclaration(t *testing.T) { + s := New(Manifest{ID: "solution-under-test", Title: "Solution Under Test"}) + s.consumed = []ConsumedModule{passthroughModule()} + if _, err := passthroughSeam(s, "https://gateway.cell.test", projectionOf(passthroughModule().As)); err != nil { + t.Fatalf("the seam refused a projection that lists the declared module: %v", err) + } +} + +// startRefusal runs the real boot entry point far enough to be refused, and +// returns what refused it. +// +// `start` is what Serve calls: it resolves the configuration, validates it, +// and only then opens a listener. A projection refusal must come from there, +// not from a test calling the validator on its own — a mutant that deleted +// `start`'s validate() call left every refusal assertion green, because each +// one validated the config itself. +func startRefusal(t *testing.T, projection string) error { + t.Helper() + bootableEnvironment(t) + t.Setenv(manifest.APIConsumesEnvironmentVariable, projection) + + s := New(Manifest{ID: "solution-under-test", Title: "Solution Under Test"}) + ln, err := s.start(context.Background()) + if ln != nil { + _ = ln.Close() + } + return err +} + +// TestEveryProjectedTargetIsDecodedNotJustTheFirst: a projection naming two +// modules decodes to both. +// +// The "several targets" case only checked that such a projection was accepted, +// which is satisfied by decoding one of them and dropping the rest: a mutant +// keeping only the first target passed it, and passed every other test, because +// nothing consumed a later one. A declaration against the SECOND target is what +// makes the difference visible. +func TestEveryProjectedTargetIsDecodedNotJustTheFirst(t *testing.T) { + first, second := "firstthing", passthroughModule().As + both := `[{"id":"a.` + first + `","module":"a","service":"` + first + + `","endpoint":"rest","protocol":"rest","as":"` + first + `"},` + + `{"id":"b.` + second + `","module":"b","service":"` + second + + `","endpoint":"rest","protocol":"rest","as":"` + second + `"}]` + + cfg := bootConfig(t, both) + if got := len(cfg.consumes.targets); got != 2 { + t.Fatalf("decoded %d targets, want 2: a projection that names two modules federates two", got) + } + if cfg.consumes.targets[1].As != second { + t.Errorf("the second target decoded as %q, want %q", cfg.consumes.targets[1].As, second) + } + + // And the later target is usable, which is what a dropped one costs: the + // declaration names only the second module, so it agrees with the + // projection exactly when that entry survived decoding. + s := New(Manifest{ID: "solution-under-test", Title: "Solution Under Test"}) + s.consumed = []ConsumedModule{passthroughModule()} + s.cfg = cfg + if _, err := s.validatePassthrough(); err != nil { + t.Fatalf("a module the projection lists second was refused, so only the first entry survived decoding: %v", err) + } +} + +// TestTheProjectionCannotBeHalfConstructed: there is one representation, so a +// config cannot hold a decoded projection that disagrees with its source. +// +// There used to be two fields — the raw string and the decoded pair — with a +// comment asking writers to set both. A config built with only the raw string +// passed validate(), because the decoded half was empty and an empty +// projection is legitimate, and the passthrough then refused every declared +// module as if nothing were projected. A comment is not a constraint; the +// second field is gone, so the state cannot be written. +func TestTheProjectionCannotBeHalfConstructed(t *testing.T) { + // Every construction goes through newProjection, so a malformed source is + // carried as an error rather than silently becoming "nothing projected". + malformed := newProjection("nope") + if malformed.err == nil { + t.Fatal("a malformed projection constructed without an error: nothing downstream can then tell it from an empty one") + } + if len(malformed.targets) != 0 { + t.Error("a malformed projection carried targets") + } + + // And the two states a reader distinguishes are distinguishable: empty + // means "consumes nothing", error means "cannot be read". + empty := newProjection("") + if empty.err != nil || len(empty.targets) != 0 { + t.Errorf("an unset projection is not empty-and-clean: targets=%d err=%v", len(empty.targets), empty.err) + } + + // A config carrying the malformed one is refused; a config carrying the + // empty one is not. That pair is what the raw string used to be able to + // contradict. + bootableEnvironment(t) + refused := loadConfig(context.Background(), "solution-under-test", nil) + refused.consumes = malformed + if err := refused.validate(); err == nil { + t.Error("a config holding a malformed projection passed validate()") + } + accepted := loadConfig(context.Background(), "solution-under-test", nil) + accepted.consumes = empty + if err := accepted.validate(); err != nil { + t.Errorf("a config holding an empty projection was refused: %v", err) + } +} diff --git a/solution.go b/solution.go index d747917..3250e98 100644 --- a/solution.go +++ b/solution.go @@ -462,10 +462,15 @@ type config struct { // so validate() must say so rather than report each empty value as // something the composition forgot to provision. environmentLoadErr error - // apiConsumes is the api.consumes projection Codefly injected, which the - // passthrough declaration and the published contract are both checked - // against before the boot listens. - apiConsumes string + // consumes is the api.consumes projection Codefly injected, decoded once at + // the boundary it enters by. There is no raw string beside it: the two were + // separate fields, and a config could then be built with only the raw one — + // which validate() passed, because the decoded pair was empty and an empty + // projection is a legitimate one, and the passthrough then refused every + // declared module as if the composition projected nothing. A second + // representation of one value is a second thing to get wrong, so there is + // one, and newProjection is the only way to make it. + consumes projection // mcp says the solution declared an MCP surface (ServeMCP), which is what // makes the two values below load-bearing: validate() checks them only // then, so a solution that serves no MCP is unaffected by a host whose @@ -622,6 +627,32 @@ func resolveGateway(ctx context.Context, module, gateway string) string { return hostAddress(ctx, module, gateway, "rest", "rest") } +// projection is the api.consumes projection as this runtime holds it: the +// decoded targets and the error decoding produced, together, because they are +// one answer about one value. It is constructed only by newProjection, so +// nothing can hold targets that disagree with the text they came from. +type projection struct { + targets []manifest.ConsumedAPI + err error +} + +// newProjection decodes the projection at the boundary it arrives on — the +// environment, for a boot, or the test seam's own argument. +// +// Whitespace alone is not a projection, and not a defect either: a render that +// emits a bare newline for a value this solution does not set means "consumes +// nothing", so it decodes to no entries rather than to a JSON error. Refusing +// that would fail a boot over a newline. Anything else that will not decode is +// a defect in what the composition projected, and the error is kept for +// validate() to refuse by name. +func newProjection(raw string) projection { + if strings.TrimSpace(raw) == "" { + return projection{} + } + targets, err := manifest.ParseConsumedAPIs(raw) + return projection{targets: targets, err: err} +} + // loadConfig resolves every address, port, and path through the Codefly SDK so // nothing is hardcoded. The host it plugs into is named by Codefly-convention // roles (overridable), and their concrete addresses are resolved from the SDK — @@ -688,7 +719,6 @@ func loadConfig(ctx context.Context, id string, environmentLoadErr error) config gatewayPeersFile: workloadPath(ctx, IdentityGatewayPeersFileEnvironmentVariable, WorkloadIdentityGatewayPeersFileKey), profile: strings.TrimSpace(env(ContractProfileEnvironmentVariable, codefly.Environment())), runtimeContext: strings.TrimSpace(env(resources.RuntimeContextPrefix, "")), - apiConsumes: env(manifest.APIConsumesEnvironmentVariable, ""), // Carried here, which the refactor that changed this function's // signature dropped: three refusals and a boot log exist to tell an // operator "the SDK resolved nothing" apart from "loading the injected @@ -723,6 +753,7 @@ func loadConfig(ctx context.Context, id string, environmentLoadErr error) config // in-cluster address in a deployment, where no public client could reach // it: that is what the declared configuration is for. The public MCP URL // needs no declaration at all, being derived from the two values above it. + cfg.consumes = newProjection(env(manifest.APIConsumesEnvironmentVariable, "")) cfg.mcpIssuerURL, cfg.mcpIssuerExplicit = resolveMCPIssuer(ctx, frontendURL) cfg.mcpPublicURL, cfg.mcpPublicExplicit = resolveMCPPublicURL(ctx, public, id) return cfg @@ -978,6 +1009,14 @@ func (c config) validate() error { return fmt.Errorf("unusable contract profile %q: %w — it is the Codefly environment's own name unless %s overrides it", c.profile, err, ContractProfileEnvironmentVariable) } + // Judged whether or not this solution declares a passthrough. The + // projection is the composition's output, so no override an operator can + // set repairs it, and the variable it arrived in is the only handle a + // refusal has. + if c.consumes.err != nil { + return fmt.Errorf("the api.consumes projection in %s cannot be decoded: %w — it is projected by the composition (`codefly run solution`), so it is fixed there; a solution that consumes nothing leaves it unset", + manifest.APIConsumesEnvironmentVariable, c.consumes.err) + } // The MCP surface's own configuration, checked here because that is where // boot configuration is checked. It is a no-op for a solution that declared // no MCP surface, which is what keeps a host whose issuer does not resolve diff --git a/solution_test.go b/solution_test.go index c483342..47366f4 100644 --- a/solution_test.go +++ b/solution_test.go @@ -1953,7 +1953,7 @@ func TestAnAskCannotBeEditedAfterItsCeilingIsChecked(t *testing.T) { Contract(ModuleContract{Ceilings: map[string]map[string][]Scope{ localProfile: {"things": {{ResourceKind: "things", Actions: []string{"read"}}}}, }}) - server.cfg = config{gatewayURL: gw.URL, profile: localProfile, apiConsumes: consumesThings} + server.cfg = config{gatewayURL: gw.URL, profile: localProfile, consumes: newProjection(consumesThings)} server.principal = testPrincipal contract, err := server.resolveContract() if err != nil { diff --git a/typed_boundary_test.go b/typed_boundary_test.go index 1179d33..0ccf283 100644 --- a/typed_boundary_test.go +++ b/typed_boundary_test.go @@ -6,6 +6,7 @@ import ( "go/types" "path/filepath" "strings" + "sync" "testing" "golang.org/x/tools/go/packages" @@ -46,9 +47,40 @@ import ( // `workcontext.WorkContextV1` — a type the SDK does not export — and supplied // three rounds of false evidence that the whole-capability rule worked. -// gatePackages loads this module's packages with full type information. +// loadedGates caches the unchanged-source load. packages.Load shells out to +// the go command, which costs seconds, and the gates that read the package as +// it is all want the same answer — the sources cannot change inside one run. +// Loading per gate put the suite over CI's five-minute budget. +var ( + loadedGatesOnce sync.Once + loadedGates []*packages.Package + loadedGatesErr error +) + +// gatePackages loads this module's packages with full type information. A nil +// overlay is the package as it is, and is loaded once; an overlay is a +// different package and is loaded on its own. func gatePackages(t *testing.T, overlay map[string][]byte) []*packages.Package { t.Helper() + if overlay == nil { + loadedGatesOnce.Do(func() { + loadedGates, loadedGatesErr = loadGatePackages(nil) + }) + if loadedGatesErr != nil { + t.Fatalf("load this module for type-resolved gating: %v", loadedGatesErr) + } + return loadedGates + } + loaded, err := loadGatePackages(overlay) + if err != nil { + t.Fatalf("load this module for type-resolved gating: %v", err) + } + return loaded +} + +// loadGatePackages is the load itself, returning its complaint rather than +// failing a test, so the cache above can hold the outcome. +func loadGatePackages(overlay map[string][]byte) ([]*packages.Package, error) { cfg := &packages.Config{ Mode: packages.NeedName | packages.NeedFiles | packages.NeedSyntax | packages.NeedTypes | packages.NeedTypesInfo | packages.NeedDeps | packages.NeedImports, @@ -56,22 +88,22 @@ func gatePackages(t *testing.T, overlay map[string][]byte) []*packages.Package { } loaded, err := packages.Load(cfg, ".", "./passthroughtest") if err != nil { - t.Fatalf("load this module for type-resolved gating: %v", err) + return nil, err } if len(loaded) < 2 { - t.Fatalf("loaded %d packages, want at least the root and passthroughtest: a gate that reads nothing passes", len(loaded)) + return nil, fmt.Errorf("loaded %d packages, want at least the root and passthroughtest: a gate that reads nothing passes", len(loaded)) } for _, pkg := range loaded { // A fixture that does not COMPILE cannot supply evidence. This is the // check the parse-only probes did not have. for _, err := range pkg.Errors { - t.Fatalf("%s does not type-check, so nothing below means anything: %v", pkg.PkgPath, err) + return nil, fmt.Errorf("%s does not type-check, so nothing below means anything: %v", pkg.PkgPath, err) } if pkg.TypesInfo == nil { - t.Fatalf("%s loaded without type information", pkg.PkgPath) + return nil, fmt.Errorf("%s loaded without type information", pkg.PkgPath) } } - return loaded + return loaded, nil } // resolved reports whether a type is a real one. go/types gives the INVALID