Fix/pointer gaps

.github/workflows/go-test.yml

@@ -14,6 +14,4 @@ on:
 jobs:
   test:
     uses: go-openapi/ci-workflows/.github/workflows/go-test.yml@befbc52f24215555cb601b0d824b2987c1d49d0b # v0.4.3
-    with:
-      extra-flags: '-tags poolsdebug'
     secrets: inherit

.github/workflows/integration-test.yml

@@ -0,0 +1,53 @@
+name: integration test
+
+permissions:
+  pull-requests: read
+  contents: read
+
+on:
+  push:
+    branches:
+      - master
+
+  pull_request:
+
+jobs:
+  test:
+    name: integration (${{ matrix.os }})
+    runs-on: ${{ matrix.os }}
+    timeout-minutes: 30
+    strategy:
+      fail-fast: false
+      matrix:
+        os: [ubuntu-latest]
+
+    steps:
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+
+      - uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
+        with:
+          go-version: stable
+          check-latest: true
+          cache: true
+          cache-dependency-path: '**/go.sum'
+
+      - name: Run integration tests
+        run: >
+          go test
+          -tags poolsdebug
+          -count 1
+          -timeout 30m
+
+  # Single gate job for branch protection rules
+  integration-test:
+    name: integration test
+    needs: test
+    if: always()
+    runs-on: ubuntu-latest
+    steps:
+      - name: Check matrix results
+        run: |
+          if [ "${{ needs.test.result }}" != "success" ]; then
+            echo "Matrix jobs failed or were cancelled"
+            exit 1
+          fi

.gitignore

@@ -3,3 +3,4 @@
 .idea
 .env
 .mcp.json
+.worktrees

.golangci.yml

@@ -7,6 +7,8 @@ linters:
     - gochecknoglobals
     - gochecknoinits
     - godox
+    - gomodguard
+    - gomodguard_v2
     - exhaustruct
     - ireturn
     - nlreturn

default_validator.go

@@ -76,8 +76,11 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
 	res := validatorPools.results.Borrow() // will redeem when merged
 	s := d.SpecValidator
 
-	for method, pathItem := range s.expandedAnalyzer().Operations() {
-		for path, op := range pathItem {
+	operations := s.expandedAnalyzer().Operations()
+	for _, method := range sortedKeys(operations) {
+		pathItem := operations[method]
+		for _, path := range sortedKeys(pathItem) {
+			op := pathItem[path]
 			// parameters
 			for _, param := range paramHelp.safeExpandedParamsFor(path, method, op.ID, res, s) {
 				if param.Default != nil && param.Required {
@@ -92,6 +95,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
 				if param.Default != nil && param.Schema == nil {
 					// check param default value is valid
 					red := newParamValidator(&param, s.KnownFormats, d.schemaOptions).Validate(param.Default) //#nosec
+					red.relocate(s.parameterPath(path, method, param.In, param.Name).child(jsonDefault))
 					if red.HasErrorsOrWarnings() {
 						res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), defaultValueDoesNotValidateMsg(param.Name, param.In))
 						res.Merge(red)
@@ -113,7 +117,7 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
 
 				if param.Schema != nil {
 					// Validate default value against schema
-					red := d.validateDefaultValueSchemaAgainstSchema(s.parameterPath(path, method, param.In, param.Name), param.In, param.Schema)
+					red := d.validateDefaultValueSchemaAgainstSchema(s.parameterPath(path, method, param.In, param.Name).structuralChild(jsonSchema), param.In, param.Schema)
 					if red.HasErrorsOrWarnings() {
 						res.addErrorsAt(s.parameterPath(path, method, param.In, param.Name), defaultValueDoesNotValidateMsg(param.Name, param.In))
 						res.Merge(red)
@@ -130,8 +134,9 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
 				}
 				// Same constraint on regular Responses
 				if op.Responses.StatusCodeResponses != nil { // Safeguard
-					for code, r := range op.Responses.StatusCodeResponses {
-						res.Merge(d.validateDefaultInResponse(&r, "response", path, method, code, op.ID)) //#nosec
+					for _, code := range sortedKeys(op.Responses.StatusCodeResponses) {
+						r := op.Responses.StatusCodeResponses[code]
+						res.Merge(d.validateDefaultInResponse(&r, "response", path, method, code, op.ID))
 					}
 				}
 			} else if op.ID != "" {
@@ -143,8 +148,10 @@ func (d *defaultValidator) validateDefaultValueValidAgainstSchema() *Result {
 	if s.spec.Spec().Definitions != nil { // Safeguard
 		// reset explored schemas to get depth-first recursive-proof exploration
 		d.resetVisited()
-		for nm, sch := range s.spec.Spec().Definitions {
-			res.Merge(d.validateDefaultValueSchemaAgainstSchema(newPathSegments(swaggerDefinitions, nm), "body", &sch)) //#nosec
+		definitions := s.spec.Spec().Definitions
+		for _, nm := range sortedKeys(definitions) {
+			sch := definitions[nm]
+			res.Merge(d.validateDefaultValueSchemaAgainstSchema(newPathSegments(swaggerDefinitions, nm), "body", &sch))
 		}
 	}
 	return res
@@ -155,20 +162,21 @@ func (d *defaultValidator) validateDefaultInResponse(
 ) *Result {
 	s := d.SpecValidator
 
-	response, res := responseHelp.expandResponseRef(resp, path, s)
+	responseName, responseCodeAsStr := responseHelp.responseMsgVariants(responseType, responseCode)
+	response, res := responseHelp.expandResponseRef(resp, path, responsePath(path, method, responseCodeAsStr), s)
 	if !res.IsValid() {
 		return res
 	}
 
-	responseName, responseCodeAsStr := responseHelp.responseMsgVariants(responseType, responseCode)
-
 	if response.Headers != nil { // Safeguard
-		for nm, h := range response.Headers {
+		for _, nm := range sortedKeys(response.Headers) {
+			h := response.Headers[nm]
 			// reset explored schemas to get depth-first recursive-proof exploration
 			d.resetVisited()
 
 			if h.Default != nil {
 				red := newHeaderValidator(nm, &h, s.KnownFormats, d.schemaOptions).Validate(h.Default) //#nosec
+				red.relocate(responseHeaderPath(path, method, responseCodeAsStr, nm).child(jsonDefault))
 				if red.HasErrorsOrWarnings() {
 					res.addErrorsAt(responseHeaderPath(path, method, responseCodeAsStr, nm), defaultValueHeaderDoesNotValidateMsg(operationID, nm, responseName))
 					res.Merge(red)
@@ -244,11 +252,13 @@ func (d *defaultValidator) validateDefaultValueSchemaAgainstSchema(path pathSegm
 		// NOTE: we keep validating values, even though additionalItems is not supported by Swagger 2.0 (and 3.0 as well)
 		res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonAdditionalItems), in, schema.AdditionalItems.Schema))
 	}
-	for propName, prop := range schema.Properties {
-		res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.structuralChild(jsonProperties).child(propName), in, &prop)) //#nosec
+	for _, propName := range sortedKeys(schema.Properties) {
+		prop := schema.Properties[propName]
+		res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.structuralChild(jsonProperties).child(propName), in, &prop))
 	}
-	for propName, prop := range schema.PatternProperties {
-		res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.structuralChild(jsonPatternProperties).child(propName), in, &prop)) //#nosec
+	for _, propName := range sortedKeys(schema.PatternProperties) {
+		prop := schema.PatternProperties[propName]
+		res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.structuralChild(jsonPatternProperties).child(propName), in, &prop))
 	}
 	if schema.AdditionalProperties != nil && schema.AdditionalProperties.Schema != nil {
 		res.Merge(d.validateDefaultValueSchemaAgainstSchema(path.child(jsonAdditionalProperties), in, schema.AdditionalProperties.Schema))

deterministic_test.go

@@ -0,0 +1,194 @@
+// SPDX-FileCopyrightText: Copyright 2015-2025 go-swagger maintainers
+// SPDX-License-Identifier: Apache-2.0
+
+package validate
+
+import (
+	"encoding/json"
+	"path/filepath"
+	"testing"
+
+	"github.com/go-openapi/loads"
+	"github.com/go-openapi/strfmt"
+	"github.com/go-openapi/testify/v2/assert"
+	"github.com/go-openapi/testify/v2/require"
+)
+
+// repeats is how many times a document is validated when checking that its
+// report does not move. Go randomises map iteration per range statement, so a
+// two-key map picks the same starting bucket often enough that a handful of
+// runs would not notice; a few dozen make an unstable report near-certain to
+// show itself.
+const repeats = 50
+
+// twoFaultyDefinitionsFixture holds one fault in each of two definitions: Pet
+// requires a property it never declares (an error), Tag marks one both required
+// and readOnly (a warning).
+//
+// Walking definitions in map order, the check stops on the first fault it meets,
+// so whether Tag was heard from at all used to depend on where the map started.
+const twoFaultyDefinitionsFixture = `{
+  "swagger": "2.0",
+  "info": {"title": "t", "version": "1"},
+  "paths": {
+    "/pets": {"get": {"operationId": "getPets",
+      "responses": {"200": {"description": "ok", "schema": {"$ref": "#/definitions/Pet"}}}}}
+  },
+  "definitions": {
+    "Pet": {"type": "object", "required": ["notDeclared"], "properties": {"name": {"type": "string"}}},
+    "Tag": {"type": "object", "required": ["readOnlyToo"], "properties": {"readOnlyToo": {"type": "string", "readOnly": true}}}
+  }
+}`
+
+// twoCircularDefinitionsFixture holds two definitions that are each their own
+// ancestor. Only one circular ancestry is reported, and which one it is used to
+// be drawn from the map iteration order.
+const twoCircularDefinitionsFixture = `{
+  "swagger": "2.0",
+  "info": {"title": "t", "version": "1"},
+  "paths": {
+    "/a": {"get": {"operationId": "getA",
+      "responses": {"200": {"description": "ok", "schema": {"$ref": "#/definitions/A"}}}}},
+    "/b": {"get": {"operationId": "getB",
+      "responses": {"200": {"description": "ok", "schema": {"$ref": "#/definitions/B"}}}}}
+  },
+  "definitions": {
+    "A": {"type": "object", "allOf": [{"$ref": "#/definitions/A"}]},
+    "B": {"type": "object", "allOf": [{"$ref": "#/definitions/B"}]}
+  }
+}`
+
+func TestDeterministic_RequiredDefinitions(t *testing.T) {
+	t.Parallel()
+
+	t.Run("stopping on the first fault always stops on the same one", func(t *testing.T) {
+		t.Parallel()
+
+		reports := repeatedReports(t, twoFaultyDefinitionsFixture, func(v *SpecValidator) {
+			v.Options.ContinueOnErrors = false
+		})
+
+		assertSameReport(t, reports)
+		assert.SliceContainsT(t, reports[0], "ERR /definitions/Pet/required/0")
+	})
+
+	t.Run("reporting everything reports both definitions", func(t *testing.T) {
+		t.Parallel()
+
+		reports := repeatedReports(t, twoFaultyDefinitionsFixture, func(v *SpecValidator) {
+			v.Options.ContinueOnErrors = true
+		})
+
+		assertSameReport(t, reports)
+		assert.SliceContainsT(t, reports[0], "ERR /definitions/Pet/required/0")
+		assert.SliceContainsT(t, reports[0], "WARN /definitions/Tag/required/0")
+	})
+}
+
+func TestDeterministic_CircularAncestry(t *testing.T) {
+	t.Parallel()
+
+	t.Run("the definition named is the first in name order", func(t *testing.T) {
+		t.Parallel()
+
+		reports := repeatedReports(t, twoCircularDefinitionsFixture, func(v *SpecValidator) {
+			v.Options.ContinueOnErrors = false
+		})
+
+		assertSameReport(t, reports)
+		assert.SliceContainsT(t, reports[0], "ERR /definitions/A")
+	})
+
+	t.Run("reporting everything reports both circular definitions", func(t *testing.T) {
+		t.Parallel()
+
+		// the check used to return on the first circular ancestry whatever the
+		// options said, so the second definition could never be heard from
+		reports := repeatedReports(t, twoCircularDefinitionsFixture, func(v *SpecValidator) {
+			v.Options.ContinueOnErrors = true
+		})
+
+		assertSameReport(t, reports)
+		assert.SliceContainsT(t, reports[0], "ERR /definitions/A")
+		assert.SliceContainsT(t, reports[0], "ERR /definitions/B")
+	})
+}
+
+// TestDeterministic_FullReport guards the whole sweep rather than the two known
+// sites: a document exercising most checks must produce the very same report,
+// in the very same order, on every run.
+func TestDeterministic_FullReport(t *testing.T) {
+	t.Parallel()
+
+	// a whole report is a much finer probe than a single finding: every map a
+	// check walks contributes to it, so a handful of runs suffice
+	const runs = 20
+
+	for _, fixture := range []string{
+		filepath.Join("fixtures", "validation", "fixture-1231.yaml"),
+		filepath.Join("fixtures", "validation", "fixture-additional-items-invalid-values.yaml"),
+		filepath.Join("fixtures", "validation", "fixture-342.yaml"),
+	} {
+		t.Run(filepath.Base(fixture), func(t *testing.T) {
+			t.Parallel()
+
+			reports := make([][]string, 0, runs)
+			for range runs {
+				doc, err := loads.Spec(fixture)
+				require.NoError(t, err)
+
+				validator := NewSpecValidator(doc.Schema(), strfmt.Default)
+				validator.Options.ContinueOnErrors = true
+				res, _ := validator.Validate(doc)
+				reports = append(reports, report(res))
+			}
+
+			require.NotEmpty(t, reports[0], "expected the fixture to yield findings")
+			assertSameReport(t, reports)
+		})
+	}
+}
+
+// repeatedReports validates the same document [repeats] times and returns one
+// report per run, each in the order the checks emitted it.
+func repeatedReports(t *testing.T, raw string, configure func(*SpecValidator)) [][]string {
+	t.Helper()
+
+	reports := make([][]string, 0, repeats)
+	for range repeats {
+		// reloading each time so that no state carried by the document or its
+		// analyzer can smooth over an unstable walk
+		doc, err := loads.Analyzed(json.RawMessage(raw), "")
+		require.NoError(t, err)
+
+		validator := NewSpecValidator(doc.Schema(), strfmt.Default)
+		configure(validator)
+		res, _ := validator.Validate(doc)
+		reports = append(reports, report(res))
+	}
+
+	return reports
+}
+
+// report renders the located findings of a result as one line each, so that two
+// runs can be compared on both what they found and where they said it was.
+func report(res *Result) []string {
+	lines := make([]string, 0, len(res.Errors)+len(res.Warnings))
+	for _, located := range res.LocatedErrors() {
+		lines = append(lines, "ERR "+located.Pointer)
+	}
+	for _, located := range res.LocatedWarnings() {
+		lines = append(lines, "WARN "+located.Pointer)
+	}
+
+	return lines
+}
+
+func assertSameReport(t *testing.T, reports [][]string) {
+	t.Helper()
+
+	require.NotEmpty(t, reports)
+	for i, got := range reports[1:] {
+		assert.Equal(t, reports[0], got, "run %d reported differently from run 0", i+1)
+	}
+}