From 123d52ebf58d57e5d931c7983712ce26d4a6cbb9 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:12:05 +0200 Subject: [PATCH 01/12] fix(auth): require https for the scheduled-task OIDC JWKS URL validateAbsoluteURL accepted any scheme, so an http JWKS endpoint let a network attacker substitute the verification keys. Require https, matching oidc.SetIssuerURL; plain http stays allowed for loopback hosts only so local test doubles keep working. The empty value still falls back to the Google JWKS URL. Closes #66 Co-Authored-By: Claude Opus 5.5 --- internal/server/scheduledauth/validator.go | 23 +++++++++++-- .../server/scheduledauth/validator_test.go | 33 +++++++++++++++++++ 2 files changed, 53 insertions(+), 3 deletions(-) diff --git a/internal/server/scheduledauth/validator.go b/internal/server/scheduledauth/validator.go index ba5a87dbe..f193cc670 100644 --- a/internal/server/scheduledauth/validator.go +++ b/internal/server/scheduledauth/validator.go @@ -8,6 +8,7 @@ import ( "fmt" "io" "log" + "net" "net/http" "net/url" "strings" @@ -142,7 +143,7 @@ func configureOIDC(v *Validator, cfg Config) (*Validator, error) { if cfg.JWKSURL == "" { cfg.JWKSURL = GoogleJWKSURL } - if err := validateAbsoluteURL(cfg.JWKSURL, "SCHEDULED_TASK_OIDC_JWKS_URL"); err != nil { + if err := validateHTTPSURL(cfg.JWKSURL, "SCHEDULED_TASK_OIDC_JWKS_URL"); err != nil { return nil, err } if len(cfg.Audiences) == 0 { @@ -214,7 +215,9 @@ func cleanSet(in []string, label string) (map[string]struct{}, error) { return out, nil } -func validateAbsoluteURL(raw, label string) error { +// validateHTTPSURL requires an absolute https URL. Plain http is allowed only +// for loopback hosts, which a network attacker cannot sit in front of. +func validateHTTPSURL(raw, label string) error { u, err := url.Parse(raw) if err != nil { return fmt.Errorf("%w: %s must be an absolute URL: %w", ErrConfigInvalid, label, err) @@ -222,7 +225,21 @@ func validateAbsoluteURL(raw, label string) error { if u.Scheme == "" || u.Host == "" { return fmt.Errorf("%w: %s must be an absolute URL", ErrConfigInvalid, label) } - return nil + if u.Scheme == "https" { + return nil + } + if u.Scheme == "http" && isLoopbackHost(u.Hostname()) { + return nil + } + return fmt.Errorf("%w: %s must use https", ErrConfigInvalid, label) +} + +func isLoopbackHost(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip := net.ParseIP(host) + return ip != nil && ip.IsLoopback() } // Mode returns the validator's auth mode. Useful for /health. diff --git a/internal/server/scheduledauth/validator_test.go b/internal/server/scheduledauth/validator_test.go index da3cc623d..b630a2b20 100644 --- a/internal/server/scheduledauth/validator_test.go +++ b/internal/server/scheduledauth/validator_test.go @@ -884,6 +884,39 @@ func TestNew_RejectsOIDCMalformedJWKSURL(t *testing.T) { } } +func TestNew_OIDCJWKSURLScheme(t *testing.T) { + cases := []struct { + name string + jwksURL string + wantErr bool + }{ + {"http remote host rejected", "http://jwks.example.com/keys", true}, + {"ftp rejected", "ftp://jwks.example.com/keys", true}, + {"https accepted", "https://jwks.example.com/keys", false}, + {"http loopback accepted for local test doubles", "http://127.0.0.1:8080/keys", false}, + {"empty falls back to Google JWKS URL", "", false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := New(Config{ + Mode: ModeOIDC, + JWKSURL: tc.jwksURL, + Audiences: []string{"https://api.example.com"}, + Subjects: []string{testSchedulerSubject}, + }) + if tc.wantErr { + if !errors.Is(err, ErrConfigInvalid) { + t.Fatalf("expected ErrConfigInvalid, got: %v", err) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + }) + } +} + func TestNew_Rejects_BearerWithoutSecret(t *testing.T) { _, err := New(Config{Mode: ModeBearer}) if !errors.Is(err, ErrConfigInvalid) { From dec204f4202283b92ed6c3218f80bd47944d4775 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:12:28 +0200 Subject: [PATCH 02/12] fix(ci): allowlist deploy job results in the deploy-all failure check The check only denylisted "failure", so a cancelled deploy job fell through to the success summary. Treat only success and skipped as ok for the four deploy jobs. Closes #196 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy-all.yml | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/.github/workflows/deploy-all.yml b/.github/workflows/deploy-all.yml index c89d51d69..ab80b48db 100644 --- a/.github/workflows/deploy-all.yml +++ b/.github/workflows/deploy-all.yml @@ -315,25 +315,19 @@ jobs: run: | FAILED=false - # determine-deployment is checked for anything other than success, not - # just "failure". When it stops the run the four deploy jobs are - # SKIPPED rather than failed, so testing only those would report "all - # deployments completed successfully" for a run that deployed nothing. - # - # The four deploy jobs are deliberately still tested for "failure" - # only. A CANCELLED run leaves them `cancelled` and reaches the - # success line, which is the same false report. That predates this - # change, so it is tracked in #1810 rather than fixed here. - # - # Note this step is the only success-by-default check in the repo. All - # four per-cloud `summary` jobs allowlist on `== "success"` instead, - # so an unenumerated result falls to their failure branch and errs - # safe. This denylist is the outlier, not the pattern. - if [[ "${{ needs.determine-deployment.result }}" != "success" ]] || \ - [[ "${{ needs.deploy-aws-lambda.result }}" == "failure" ]] || \ - [[ "${{ needs.deploy-aws-fargate.result }}" == "failure" ]] || \ - [[ "${{ needs.deploy-gcp.result }}" == "failure" ]] || \ - [[ "${{ needs.deploy-azure.result }}" == "failure" ]]; then + # Allowlist, not denylist: determine-deployment must be "success" and + # each deploy job "success" or "skipped" (not selected). Anything else, + # including "cancelled", counts as a failed run. + for result in \ + "${{ needs.deploy-aws-lambda.result }}" \ + "${{ needs.deploy-aws-fargate.result }}" \ + "${{ needs.deploy-gcp.result }}" \ + "${{ needs.deploy-azure.result }}"; do + if [[ "$result" != "success" && "$result" != "skipped" ]]; then + FAILED=true + fi + done + if [[ "${{ needs.determine-deployment.result }}" != "success" ]]; then FAILED=true fi From c131314a42d0a0b4774cd9013373bd9fc629a2df Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:13:17 +0200 Subject: [PATCH 03/12] fix(api): make two openapi schemas valid for the declared 3.0.3 dialect last_collection_partial used nullable without a type, and PlanRequest.max_step_commitment used a numeric exclusiveMinimum. Add type: object and use minimum: 0 with boolean exclusiveMinimum, keeping the nullable payload and strictly-positive cap semantics. A spec test pins both. Closes #1075 Co-Authored-By: Claude Opus 5.5 --- internal/api/openapi.yaml | 4 ++- internal/api/openapi_dialect_test.go | 37 ++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 internal/api/openapi_dialect_test.go diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index f009e1b5f..bdff376df 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -3500,6 +3500,7 @@ components: nullable: true description: Set while a collection is in flight. last_collection_partial: + type: object nullable: true description: > Non-null when the most recent persisted collection kept rows from @@ -3576,7 +3577,8 @@ components: max_step_commitment: type: number nullable: true - exclusiveMinimum: 0 + minimum: 0 + exclusiveMinimum: true description: >- Ceiling on the total commitment (upfront + monthly * term) one ramp step may auto-purchase. Omit on create for no cap (auto-purchase of diff --git a/internal/api/openapi_dialect_test.go b/internal/api/openapi_dialect_test.go new file mode 100644 index 000000000..e25479656 --- /dev/null +++ b/internal/api/openapi_dialect_test.go @@ -0,0 +1,37 @@ +package api + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gopkg.in/yaml.v3" +) + +// Issue #1075: the spec declares OpenAPI 3.0.3, where nullable needs an +// explicit type and exclusiveMinimum is a boolean paired with minimum. +func TestOpenAPIDialect303Schemas(t *testing.T) { + type property struct { + Type string `yaml:"type"` + Nullable bool `yaml:"nullable"` + Minimum *int `yaml:"minimum"` + ExclusiveMinimum any `yaml:"exclusiveMinimum"` + } + var spec struct { + Components struct { + Schemas map[string]struct { + Properties map[string]property `yaml:"properties"` + } `yaml:"schemas"` + } `yaml:"components"` + } + require.NoError(t, yaml.Unmarshal(openapiSpec, &spec)) + + partial := spec.Components.Schemas["RecommendationsFreshness"].Properties["last_collection_partial"] + assert.True(t, partial.Nullable) + assert.Equal(t, "object", partial.Type, "nullable requires an explicit type in 3.0.3") + + stepCap := spec.Components.Schemas["PlanRequest"].Properties["max_step_commitment"] + require.NotNil(t, stepCap.Minimum) + assert.Equal(t, 0, *stepCap.Minimum) + assert.Equal(t, true, stepCap.ExclusiveMinimum, "exclusiveMinimum must be boolean in 3.0.3") +} From 1f71ac6284b9c20dda85fb8afb78e50de161f9d5 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:14:47 +0200 Subject: [PATCH 04/12] fix(iac): point AKS DB_PASSWORD_SECRET at the Secret's real key The Deployment read key "password", but the database Secret only defines "password_secret_name", so pods would fail to start once the Kubernetes resources are enabled. The CronJobs share the same env list. Closes #1115 Co-Authored-By: Claude Opus 5.5 --- terraform/modules/compute/azure/aks/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/modules/compute/azure/aks/main.tf b/terraform/modules/compute/azure/aks/main.tf index 475cf7fb4..07b051e67 100644 --- a/terraform/modules/compute/azure/aks/main.tf +++ b/terraform/modules/compute/azure/aks/main.tf @@ -398,7 +398,7 @@ resource "kubernetes_deployment" "app" { value_from { secret_key_ref { name = kubernetes_secret.database[0].metadata[0].name - key = "password" + key = "password_secret_name" } } } From 9f2c4999c898a256f69fbb0d723a948a1c7a1dcf Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:15:07 +0200 Subject: [PATCH 05/12] docs(iac): fix the lambda function_url_auth_type description It told operators to pair the variable with a cloudfront_distribution_arn input that this module does not have. State what the module actually does: it only creates the public invoke permission for NONE, and the caller grants CloudFront access for AWS_IAM, as environments/aws/compute.tf does. Closes #130 Co-Authored-By: Claude Opus 5.5 --- terraform/modules/compute/aws/lambda/variables.tf | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/terraform/modules/compute/aws/lambda/variables.tf b/terraform/modules/compute/aws/lambda/variables.tf index 8a8cf1ed8..9f57ae23a 100644 --- a/terraform/modules/compute/aws/lambda/variables.tf +++ b/terraform/modules/compute/aws/lambda/variables.tf @@ -123,8 +123,10 @@ variable "function_url_auth_type" { unreachable without a valid AWS identity. Use "NONE" only in environments where direct access without CloudFront is acceptable (e.g. local dev, or environments not yet provisioned with CloudFront). - Pair with cloudfront_distribution_arn when switching to AWS_IAM so the module can - create the necessary resource-based policy granting CloudFront invocation rights. + This module only creates the public lambda:InvokeFunctionUrl permission for "NONE". + With "AWS_IAM" the caller must grant invocation itself, e.g. an + aws_lambda_permission for principal cloudfront.amazonaws.com scoped to the + distribution ARN (see environments/aws/compute.tf). EOT type = string default = "AWS_IAM" From 8a95182974bf3c3ccb04c8b790ddf8d467541ffc Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:15:15 +0200 Subject: [PATCH 06/12] docs(iac): correct the Key Vault ACL comment in the azure environment The comment said the Container App subnet reaches the vault, but the code passes the private subnet, which exists only when create_private_subnet is true (default false). Comment-only change. Closes #288 Co-Authored-By: Claude Opus 5.5 --- terraform/environments/azure/secrets.tf | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/terraform/environments/azure/secrets.tf b/terraform/environments/azure/secrets.tf index f26213a85..055dfeba6 100644 --- a/terraform/environments/azure/secrets.tf +++ b/terraform/environments/azure/secrets.tf @@ -30,7 +30,9 @@ module "secrets" { soft_delete_retention_days = var.soft_delete_retention_days purge_protection_enabled = var.purge_protection_enabled # Default-deny network ACL: only ip_addresses in var.allowed_ip_addresses and - # the Container App subnet reach the Key Vault data plane. Dev environments + # the private subnet reach the Key Vault data plane. The private subnet + # exists only when create_private_subnet = true (default false), so by + # default no subnet is allowlisted. Dev environments # that need cross-network access from an operator laptop should add the IP # explicitly; they should not widen this to "Allow". default_network_acl_action = var.key_vault_default_network_acl_action From 8a56938265cd4d437cdee53196c766e93de19780 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:30:37 +0200 Subject: [PATCH 07/12] docs(known-issues): move resolved OpenAPI dialect doc Refs #1075 Co-Authored-By: Claude Opus 5.5 --- known_issues/{ => resolved}/38_openapi_contract.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) rename known_issues/{ => resolved}/38_openapi_contract.md (66%) diff --git a/known_issues/38_openapi_contract.md b/known_issues/resolved/38_openapi_contract.md similarity index 66% rename from known_issues/38_openapi_contract.md rename to known_issues/resolved/38_openapi_contract.md index 076cb82cd..b77e2e4ce 100644 --- a/known_issues/38_openapi_contract.md +++ b/known_issues/resolved/38_openapi_contract.md @@ -1,6 +1,6 @@ # Known Issues: OpenAPI dialect validation -> **Audit status (2026-10-18):** `1 needs triage · 0 resolved` +> **Audit status (2026-10-18):** `0 needs triage · 1 resolved` ## LOW: two existing schemas fail OpenAPI 3.0.3 structural validation @@ -16,4 +16,6 @@ from the insurance contract documentation. **Why deferred:** dialect corrections affect unrelated contracts and are tracked separately to keep the insurance documentation change focused. -**Status:** Open. Tracked in #1075. +**Issue:** [#1075](https://github.com/LeanerCloud/cloud-commitments-platform/issues/1075) + +**Status:** Fixed in this change: `last_collection_partial` has an explicit `type: object`, and `max_step_commitment` uses `minimum: 0` with boolean `exclusiveMinimum`. `TestOpenAPIDialect303Schemas` pins both. From 58a049559b5d4f29eb79721253c6af4727bbe26d Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:30:52 +0200 Subject: [PATCH 08/12] test(auth): cover loopback lookalikes in the JWKS URL check Refs #66 Co-Authored-By: Claude Opus 5.5 --- internal/server/scheduledauth/validator_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/internal/server/scheduledauth/validator_test.go b/internal/server/scheduledauth/validator_test.go index b630a2b20..00eaf5a09 100644 --- a/internal/server/scheduledauth/validator_test.go +++ b/internal/server/scheduledauth/validator_test.go @@ -894,6 +894,13 @@ func TestNew_OIDCJWKSURLScheme(t *testing.T) { {"ftp rejected", "ftp://jwks.example.com/keys", true}, {"https accepted", "https://jwks.example.com/keys", false}, {"http loopback accepted for local test doubles", "http://127.0.0.1:8080/keys", false}, + {"localhost subdomain rejected", "http://localhost.evil.com/keys", true}, + {"loopback userinfo trick rejected", "http://127.0.0.1@evil.com/keys", true}, + {"localhost trailing dot rejected", "http://localhost./x", true}, + {"loopback lookalike domain rejected", "http://127.0.0.1.nip.io/keys", true}, + {"http ipv6 loopback accepted", "http://[::1]:8080", false}, + {"http LOCALHOST accepted", "http://LOCALHOST", false}, + {"http 127.0.0.2 accepted", "http://127.0.0.2", false}, {"empty falls back to Google JWKS URL", "", false}, } for _, tc := range cases { From d49b3da4c6d728d7c7318476581dbef3c4030c73 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 18:33:07 +0200 Subject: [PATCH 09/12] fix(ci): require success for selected deploy jobs in deploy-all A job selected by determine-deployment that ends skipped now fails the run; skipped is only ok for unselected jobs. Refs #196 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/deploy-all.yml | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/.github/workflows/deploy-all.yml b/.github/workflows/deploy-all.yml index ab80b48db..103e37a6a 100644 --- a/.github/workflows/deploy-all.yml +++ b/.github/workflows/deploy-all.yml @@ -315,18 +315,22 @@ jobs: run: | FAILED=false - # Allowlist, not denylist: determine-deployment must be "success" and - # each deploy job "success" or "skipped" (not selected). Anything else, - # including "cancelled", counts as a failed run. - for result in \ - "${{ needs.deploy-aws-lambda.result }}" \ - "${{ needs.deploy-aws-fargate.result }}" \ - "${{ needs.deploy-gcp.result }}" \ - "${{ needs.deploy-azure.result }}"; do - if [[ "$result" != "success" && "$result" != "skipped" ]]; then - FAILED=true + # Allowlist, not denylist: determine-deployment must be "success". A + # selected deploy job must be "success"; "skipped" is only ok for a + # job that was not selected. Anything else, including "cancelled", + # counts as a failed run. + check_job() { + local selected="$1" result="$2" + if [[ "$selected" == "true" ]]; then + [[ "$result" == "success" ]] + else + [[ "$result" == "skipped" || "$result" == "success" ]] fi - done + } + check_job "${{ needs.determine-deployment.outputs.deploy-aws-lambda }}" "${{ needs.deploy-aws-lambda.result }}" || FAILED=true + check_job "${{ needs.determine-deployment.outputs.deploy-aws-fargate }}" "${{ needs.deploy-aws-fargate.result }}" || FAILED=true + check_job "${{ needs.determine-deployment.outputs.deploy-gcp }}" "${{ needs.deploy-gcp.result }}" || FAILED=true + check_job "${{ needs.determine-deployment.outputs.deploy-azure }}" "${{ needs.deploy-azure.result }}" || FAILED=true if [[ "${{ needs.determine-deployment.result }}" != "success" ]]; then FAILED=true fi From 67e323d7b9384ce02dcd70312385683a18e95fee Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 21:35:06 +0200 Subject: [PATCH 10/12] test(auth): reject non-loopback IP literals in the JWKS URL check Refs #66 Co-Authored-By: Claude Opus 5.5 --- internal/server/scheduledauth/validator_test.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/internal/server/scheduledauth/validator_test.go b/internal/server/scheduledauth/validator_test.go index 00eaf5a09..156990b6e 100644 --- a/internal/server/scheduledauth/validator_test.go +++ b/internal/server/scheduledauth/validator_test.go @@ -901,6 +901,9 @@ func TestNew_OIDCJWKSURLScheme(t *testing.T) { {"http ipv6 loopback accepted", "http://[::1]:8080", false}, {"http LOCALHOST accepted", "http://LOCALHOST", false}, {"http 127.0.0.2 accepted", "http://127.0.0.2", false}, + {"http unspecified ip rejected", "http://0.0.0.0/k", true}, + {"http private ip rejected", "http://10.0.0.1/k", true}, + {"http link-local metadata ip rejected", "http://169.254.169.254/k", true}, {"empty falls back to Google JWKS URL", "", false}, } for _, tc := range cases { From 1b1e4b9b3e16b2036e04309ef66685398fe77636 Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Mon, 19 Oct 2026 21:36:44 +0200 Subject: [PATCH 11/12] test(iac): assert the AKS DB password secret key exists in the Secret Refs #1115 Co-Authored-By: Claude Opus 5.5 --- .../compute/azure/aks_hardening_test.go | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/terraform/modules/compute/azure/aks_hardening_test.go b/terraform/modules/compute/azure/aks_hardening_test.go index dd914efdf..6621b4cb2 100644 --- a/terraform/modules/compute/azure/aks_hardening_test.go +++ b/terraform/modules/compute/azure/aks_hardening_test.go @@ -54,3 +54,33 @@ func TestAKSClusterIsPrivateWithEntraRBAC(t *testing.T) { t.Error(`var.authorized_ip_ranges must validate against "0.0.0.0/0", so disabling the private cluster can't silently reopen the API server to the whole internet`) } } + +// Issue #1115: DB_PASSWORD_SECRET reads a secret_key_ref key from the +// database Kubernetes Secret; a key renamed in the Secret's data map but not +// in the env reference makes the pod fail with CreateContainerConfigError. +func TestAKSDatabaseSecretKeyRefsExistInSecretData(t *testing.T) { + data, err := os.ReadFile("aks/main.tf") + if err != nil { + t.Fatalf("reading aks/main.tf: %v", err) + } + content := string(data) + + secret := regexp.MustCompile(`(?s)resource\s+"kubernetes_secret"\s+"database"\s*\{.*?\n\s*data\s*=\s*\{(.*?)\n\s*\}`).FindStringSubmatch(content) + if secret == nil { + t.Fatal(`could not isolate the kubernetes_secret.database data map`) + } + keys := map[string]bool{} + for _, m := range regexp.MustCompile(`(?m)^\s*([A-Za-z_][A-Za-z0-9_]*)\s*=`).FindAllStringSubmatch(secret[1], -1) { + keys[m[1]] = true + } + + refs := regexp.MustCompile(`secret_key_ref\s*\{\s*name\s*=\s*kubernetes_secret\.database\[0\]\.metadata\[0\]\.name\s*key\s*=\s*"([^"]+)"`).FindAllStringSubmatch(content, -1) + if len(refs) == 0 { + t.Fatal(`found no secret_key_ref pointing at kubernetes_secret.database`) + } + for _, r := range refs { + if !keys[r[1]] { + t.Errorf("secret_key_ref key %q is not defined in kubernetes_secret.database data (keys: %v)", r[1], keys) + } + } +} From 9c49846e165862477d1f9f74f6c51ff2eb4718ec Mon Sep 17 00:00:00 2001 From: Cristian Magherusan-Stanciu Date: Tue, 20 Oct 2026 01:41:24 +0200 Subject: [PATCH 12/12] fix(api): accept null last_collection_partial under OpenAPI 3.0.3 Refs #1075 Co-Authored-By: Claude Opus 5.5 --- internal/api/openapi.yaml | 14 +++++----- internal/api/openapi_dialect_test.go | 38 ++++++++++++++++++---------- 2 files changed, 31 insertions(+), 21 deletions(-) diff --git a/internal/api/openapi.yaml b/internal/api/openapi.yaml index bdff376df..8f008e464 100644 --- a/internal/api/openapi.yaml +++ b/internal/api/openapi.yaml @@ -3500,17 +3500,15 @@ components: nullable: true description: Set while a collection is in flight. last_collection_partial: - type: object - nullable: true - description: > - Non-null when the most recent persisted collection kept rows from - at least one incomplete sweep. Cleared by the next collection whose - sweeps are all complete. Carries counts and reason categories only. - allOf: - - $ref: '#/components/schemas/RecommendationsCollectionPartial' + $ref: '#/components/schemas/RecommendationsCollectionPartial' RecommendationsCollectionPartial: type: object + nullable: true + description: > + Non-null when the most recent persisted collection kept rows from + at least one incomplete sweep. Cleared by the next collection whose + sweeps are all complete. Carries counts and reason categories only. required: [providers] properties: providers: diff --git a/internal/api/openapi_dialect_test.go b/internal/api/openapi_dialect_test.go index e25479656..703ba41cb 100644 --- a/internal/api/openapi_dialect_test.go +++ b/internal/api/openapi_dialect_test.go @@ -1,6 +1,7 @@ package api import ( + "strings" "testing" "github.com/stretchr/testify/assert" @@ -9,29 +10,40 @@ import ( ) // Issue #1075: the spec declares OpenAPI 3.0.3, where nullable needs an -// explicit type and exclusiveMinimum is a boolean paired with minimum. +// explicit type, $ref siblings are ignored and exclusiveMinimum is a boolean +// paired with minimum. func TestOpenAPIDialect303Schemas(t *testing.T) { - type property struct { - Type string `yaml:"type"` - Nullable bool `yaml:"nullable"` - Minimum *int `yaml:"minimum"` - ExclusiveMinimum any `yaml:"exclusiveMinimum"` + type schema struct { + Ref string `yaml:"$ref"` + Type string `yaml:"type"` + Nullable bool `yaml:"nullable"` + Required []string `yaml:"required"` + Properties map[string]schema `yaml:"properties"` + Minimum *int `yaml:"minimum"` + ExclusiveMinimum any `yaml:"exclusiveMinimum"` } var spec struct { Components struct { - Schemas map[string]struct { - Properties map[string]property `yaml:"properties"` - } `yaml:"schemas"` + Schemas map[string]schema `yaml:"schemas"` } `yaml:"components"` } require.NoError(t, yaml.Unmarshal(openapiSpec, &spec)) + schemas := spec.Components.Schemas - partial := spec.Components.Schemas["RecommendationsFreshness"].Properties["last_collection_partial"] - assert.True(t, partial.Nullable) + prop := schemas["RecommendationsFreshness"].Properties["last_collection_partial"] + require.NotEmpty(t, prop.Ref, "3.0.3 ignores $ref siblings, so the property must be a bare $ref") + partial := schemas[strings.TrimPrefix(prop.Ref, "#/components/schemas/")] + + // null is accepted only if the resolved schema is nullable with a type. + assert.True(t, partial.Nullable, "null last_collection_partial must validate") assert.Equal(t, "object", partial.Type, "nullable requires an explicit type in 3.0.3") + // {"providers": []} satisfies required + type. + assert.Equal(t, []string{"providers"}, partial.Required) + assert.Equal(t, "array", partial.Properties["providers"].Type) - stepCap := spec.Components.Schemas["PlanRequest"].Properties["max_step_commitment"] + stepCap := schemas["PlanRequest"].Properties["max_step_commitment"] require.NotNil(t, stepCap.Minimum) + // minimum 0 with exclusiveMinimum true: positive accepted, 0 rejected. assert.Equal(t, 0, *stepCap.Minimum) - assert.Equal(t, true, stepCap.ExclusiveMinimum, "exclusiveMinimum must be boolean in 3.0.3") + assert.Equal(t, true, stepCap.ExclusiveMinimum, "zero must be rejected; exclusiveMinimum must be boolean in 3.0.3") }