Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 17 additions & 19 deletions .github/workflows/deploy-all.yml
Original file line number Diff line number Diff line change
Expand Up @@ -315,25 +315,23 @@ 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". 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
}
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

Expand Down
16 changes: 8 additions & 8 deletions internal/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -3500,16 +3500,15 @@ components:
nullable: true
description: Set while a collection is in flight.
last_collection_partial:
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:
Expand Down Expand Up @@ -3576,7 +3575,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
Expand Down
49 changes: 49 additions & 0 deletions internal/api/openapi_dialect_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package api

import (
"strings"
"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, $ref siblings are ignored and exclusiveMinimum is a boolean
// paired with minimum.
func TestOpenAPIDialect303Schemas(t *testing.T) {
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]schema `yaml:"schemas"`
} `yaml:"components"`
}
require.NoError(t, yaml.Unmarshal(openapiSpec, &spec))
schemas := spec.Components.Schemas

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 := 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, "zero must be rejected; exclusiveMinimum must be boolean in 3.0.3")
}
23 changes: 20 additions & 3 deletions internal/server/scheduledauth/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io"
"log"
"net"
"net/http"
"net/url"
"strings"
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -214,15 +215,31 @@ 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)
}
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.
Expand Down
43 changes: 43 additions & 0 deletions internal/server/scheduledauth/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -884,6 +884,49 @@ 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},
{"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},
{"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 {
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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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

Expand All @@ -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.
4 changes: 3 additions & 1 deletion terraform/environments/azure/secrets.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions terraform/modules/compute/aws/lambda/variables.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion terraform/modules/compute/azure/aks/main.tf
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
}
}
Expand Down
30 changes: 30 additions & 0 deletions terraform/modules/compute/azure/aks_hardening_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
}
Loading