From f5d367a0b8feb2222ed24023b0e5b45819f56916 Mon Sep 17 00:00:00 2001 From: Luca Candela Date: Wed, 28 Oct 2026 11:36:42 -0700 Subject: [PATCH 1/2] test(admission): address concurrency review findings --- admission/concurrency_test.go | 24 +++++++++++++------ .../concurrency_admission_test.go.txt | 9 ++++--- docs/admission.md | 12 ++++++++-- docs/examples/concurrency/main.go.txt | 1 + 4 files changed, 34 insertions(+), 12 deletions(-) diff --git a/admission/concurrency_test.go b/admission/concurrency_test.go index 2f39262b1..4f4f7d036 100644 --- a/admission/concurrency_test.go +++ b/admission/concurrency_test.go @@ -8,6 +8,7 @@ import ( "testing/synctest" "time" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/CaliLuke/loom/admission" @@ -100,7 +101,7 @@ func TestConcurrencyCancellationRetainsPermit(t *testing.T) { }) go func() { _, err := ep(ctx, nil) - require.ErrorIs(t, err, context.Canceled) + assert.ErrorIs(t, err, context.Canceled) }() synctest.Wait() cancel() @@ -142,7 +143,7 @@ func TestConcurrencySimultaneousCalls(t *testing.T) { <-start _, err := ep(context.Background(), nil) if err != nil { - requireOverloaded(t, err) + assertOverloaded(t, err) } }() } @@ -164,7 +165,7 @@ func TestConcurrencyResizeRetainsActiveCalls(t *testing.T) { start := func(i int) { go func() { _, err := ep(context.Background(), i) - require.NoError(t, err) + assert.NoError(t, err) }() synctest.Wait() } @@ -208,7 +209,7 @@ func TestConcurrencyUpdateCompletionAndCancellationRace(t *testing.T) { }) workers.Go(func() { _, err := ep(ctx, "held") - require.NoError(t, err) + assert.NoError(t, err) }) <-entered require.Equal(t, 1, c.Snapshot().InFlight) @@ -217,12 +218,12 @@ func TestConcurrencyUpdateCompletionAndCancellationRace(t *testing.T) { for i := range 100 { workers.Go(func() { <-start - require.NoError(t, c.SetCapacity(i%4+1)) + assert.NoError(t, c.SetCapacity(i%4+1)) _, err := ep(ctx, nil) if err != nil && !errors.Is(err, context.Canceled) { - requireOverloaded(t, err) + assertOverloaded(t, err) } - require.Error(t, c.SetCapacity(-1)) + assert.Error(t, c.SetCapacity(-1)) }) } workers.Go(func() { @@ -250,3 +251,12 @@ func requireOverloaded(t *testing.T, err error) { require.Equal(t, "overloaded", serviceError.Name) require.True(t, serviceError.Temporary) } + +func assertOverloaded(t *testing.T, err error) { + t.Helper() + var serviceError *loom.ServiceError + if assert.ErrorAs(t, err, &serviceError) { + assert.Equal(t, "overloaded", serviceError.Name) + assert.True(t, serviceError.Temporary) + } +} diff --git a/codegen/generator/testdata/concurrency_admission_test.go.txt b/codegen/generator/testdata/concurrency_admission_test.go.txt index 176fdb0cc..c769d1ed9 100644 --- a/codegen/generator/testdata/concurrency_admission_test.go.txt +++ b/codegen/generator/testdata/concurrency_admission_test.go.txt @@ -17,6 +17,7 @@ import ( "github.com/CaliLuke/loom/admission" loomhttp "github.com/CaliLuke/loom/http" loom "github.com/CaliLuke/loom/pkg" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" @@ -77,8 +78,8 @@ func TestGeneratedAdmission(t *testing.T) { defer cancel() go func() { result, err := endpoints.Run(ctx, &work.RunPayload{Key: "valid", Seconds: 1}) - require.NoError(t, err) - require.Equal(t, "done", result) + assert.NoError(t, err) + assert.Equal(t, "done", result) }() synctest.Wait() deadline, ok := ctx.Deadline() @@ -133,7 +134,9 @@ func TestGeneratedChecksRepeatInsidePermit(t *testing.T) { t.Run(mutation, func(t *testing.T) { budget, err := admission.NewConcurrency(1) require.NoError(t, err) - service := &testService{workService: workService{key: "valid"}} + finish := make(chan struct{}) + close(finish) + service := &testService{workService: workService{key: "valid"}, finish: finish} ctx, cancel := context.WithCancel(context.Background()) defer cancel() mutate := func(next loom.Endpoint) loom.Endpoint { diff --git a/docs/admission.md b/docs/admission.md index a3a16e83f..0d5ce8f93 100644 --- a/docs/admission.md +++ b/docs/admission.md @@ -120,14 +120,22 @@ go mod tidy EXAMPLE_API_KEY=local-demo-key go run . ``` -From another terminal, start two long calls, then send a third while both are -active: +From another terminal, start two long calls: ```sh curl -s -H 'X-API-Key: local-demo-key' -H 'Content-Type: application/json' \ -d '{"seconds":30}' http://127.0.0.1:8080/work & curl -s -H 'X-API-Key: local-demo-key' -H 'Content-Type: application/json' \ -d '{"seconds":30}' http://127.0.0.1:8080/work & +``` + +Wait until the server terminal has printed `work started; holding an admission +permit` twice. Those messages confirm that both calls acquired permits. Then, +before the 30-second calls finish, send the third request. If either call has +already finished, run `wait` in the request terminal to let both calls finish, +then start the two long calls again and wait for two new messages. + +```sh curl -i -H 'X-API-Key: local-demo-key' -H 'Content-Type: application/json' \ -d '{"seconds":1}' http://127.0.0.1:8080/work wait diff --git a/docs/examples/concurrency/main.go.txt b/docs/examples/concurrency/main.go.txt index 6eb7ed5ff..5576a114f 100644 --- a/docs/examples/concurrency/main.go.txt +++ b/docs/examples/concurrency/main.go.txt @@ -46,6 +46,7 @@ func main() { } func (*workService) Run(ctx context.Context, payload *work.RunPayload) (string, error) { + log.Print("work started; holding an admission permit") timer := time.NewTimer(time.Duration(payload.Seconds) * time.Second) defer timer.Stop() select { From 5f88142c7059139b1cf9e83a305d7c77ff3db4b8 Mon Sep 17 00:00:00 2001 From: Luca Candela Date: Wed, 28 Oct 2026 11:39:00 -0700 Subject: [PATCH 2/2] docs(security): address scope contract review findings --- .../references/formal-models.md | 3 +-- expr/security_scopes.go | 22 +++++++++---------- expr/tla/security_requirements/README.md | 4 ++-- internal/openapiimport/model.go | 5 +++-- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/.agents/skills/loom-framework/references/formal-models.md b/.agents/skills/loom-framework/references/formal-models.md index 4e3cb1390..6a0584fb0 100644 --- a/.agents/skills/loom-framework/references/formal-models.md +++ b/.agents/skills/loom-framework/references/formal-models.md @@ -274,6 +274,7 @@ Rendered schema validation and actual decoder behavior are separate obligations. | gRPC aggregate error status and detail ownership | [Error contracts](../../../../grpc/tla/error_contract/README.md); bounded branch-order and complete-failure checks, with wrappers and generated unary/streaming mappings tested in Go | | Nearest raw forwarding boundary and prefix independence | [Forwarded client trust](../../../../http/tla/forwarded_client/README.md); bounded chains, with IP parsing and repeated-header grouping checked in Go | | Per-scheme security scopes and roles with AND/OR/anonymous alternatives | [Security requirements](../../../../expr/tla/security_requirements/README.md); flattened-name counterexample and generated credential checks | +| OAuth flow scope inventories with independent defaults and overrides | [OAuth flow inventories](../../../../expr/tla/oauth_flow_scopes/README.md); flattening counterexample and generated callback checks | | Authentication alternatives, optional credential extraction and failed-context isolation | [Credential alternatives](../../../../internal/securitygen/tla/credential_alternatives/README.md); finite extraction/control model, with actual framing and HTTP/JSON-RPC/gRPC wire regressions | | gRPC pre-frame admission, complete application input, original cancellation and middleware rechecks; ordered AND/OR contexts and failed-branch reset | [Stream admission](../../../../security/tla/stream_admission/README.md); positive and deliberate negative models paired with runtime and generated transport regressions | | Scalar query/header/cookie absence, defaults and conversion admission | [Scalar binding presence](../../../../http/codegen/tla/scalar_binding_presence/README.md); supplied-empty counterexample, shared decoder and mounted HTTP/JSON-RPC controls | @@ -357,8 +358,6 @@ records optional/fallback admission and 3.1 projection. Go tests cover reference resolution and graph retention outside its Boolean assumptions. | JSON-RPC envelope ID source, presence and validation ordering | [Envelope ID admission](../../../../http/codegen/tla/jsonrpc_id_admission/README.md); params-first and late-handler counterexamples, shared carrier/CLI validation and mounted unary/SSE/WebSocket controls | -OAuth flow inventories: [independent defaults and overrides](../../../../expr/tla/oauth_flow_scopes/README.md), with a flattening counterexample and generated callback checks. - Schema reference siblings: [conjunction and declaration storage](../../../../internal/openapiimport/tla/reference_siblings/README.md), with rejection, replacement, mutation and occurrence-storage counterexamples. The [reusable-reference model](../../../../internal/openapiimport/tla/reusable_references/README.md) diff --git a/expr/security_scopes.go b/expr/security_scopes.go index 51a11adcd..86d1dd468 100644 --- a/expr/security_scopes.go +++ b/expr/security_scopes.go @@ -22,17 +22,6 @@ func (s *SecurityExpr) RequiredScopes(scheme *SchemeExpr) []string { return scopes } -func dupSchemeRequirements(source map[string][]string) map[string][]string { - if source == nil { - return nil - } - result := make(map[string][]string, len(source)) - for scheme, names := range source { - result[scheme] = slices.Clone(names) - } - return result -} - // FlowScopes returns a flow's effective scope inventory. A nil flow inventory // inherits the scheme defaults; a nonnil empty inventory remains empty. The // returned scope declarations are independently owned. Repeated names retain @@ -82,6 +71,17 @@ func (s *SchemeExpr) ScopeNames() []string { return names } +func dupSchemeRequirements(source map[string][]string) map[string][]string { + if source == nil { + return nil + } + result := make(map[string][]string, len(source)) + for scheme, names := range source { + result[scheme] = slices.Clone(names) + } + return result +} + func dupScopeDeclarations(source []*ScopeExpr) []*ScopeExpr { if source == nil { return nil diff --git a/expr/tla/security_requirements/README.md b/expr/tla/security_requirements/README.md index 793ef2059..49aa6c0f5 100644 --- a/expr/tla/security_requirements/README.md +++ b/expr/tla/security_requirements/README.md @@ -5,7 +5,7 @@ AND branches (including empty/anonymous branches), and every per-scheme grant set. `checked.cfg` preserves each scheme's required names and passed 4,096 states (8,192 generated) with TLC 1.7.4. `flatten.cfg` deliberately combines requirements across a conjunction; its initial-state counterexample requires -no grants from scheme a and read from b, but incorrectly requires read from a. +no grants from scheme `a` and `read` from `b`, but incorrectly requires `read` from `a`. This compares design candidates; the prior importer explicitly rejected unequal lists rather than admitting that faulty projection. @@ -15,7 +15,7 @@ Run explicitly from this directory with Java and the installed TLA+ 1.7.4 jar: java -XX:+UseParallelGC -cp "$TLA_JAR" tlc2.TLC -workers 1 -config checked.cfg -metadir /tmp/loom-security-requirements Requirements.tla ``` -Substitute `flatten.cfg` and a separate temporary metadir for the negative case. +Substitute `flatten.cfg` and a separate temporary `-metadir` for the negative case. This is an opt-in design tool, never a build, hook, CI or release gate. `expr.SecurityExpr.SchemeRequirements` owns explicit names by scheme identity. diff --git a/internal/openapiimport/model.go b/internal/openapiimport/model.go index 7e0a8ae25..c3f63149e 100644 --- a/internal/openapiimport/model.go +++ b/internal/openapiimport/model.go @@ -133,8 +133,9 @@ type SecurityRequirement struct { type SecurityRequirementScheme struct { // Name is the referenced security scheme component key. Name string - // Scopes contains the authored scopes. Supported API-key schemes require - // this list to be empty. + // Scopes contains authored OAuth scopes or, for non-OAuth schemes in + // OpenAPI 3.1 and 3.2, application roles. Non-OAuth requirements in + // OpenAPI 3.0 must leave this list empty. Scopes []string }