From 10dfc33a86d6a212b94df140a898b31ac2039db0 Mon Sep 17 00:00:00 2001 From: Tristan Cartledge Date: Thu, 28 May 2026 13:38:53 +1000 Subject: [PATCH] fix: improve openapi lint diagnostics and CI portability --- .mise-tasks/lint | 15 ++++++---- cmd/update-examples/main.go | 4 +-- cmd/update-lint-docs/main.go | 6 ++-- hashing/hashing.go | 2 +- internal/testutils/utils.go | 6 ++-- linter/format/text.go | 2 +- openapi/index.go | 23 ++++++++++++++- openapi/linter/linter_test.go | 53 +++++++++++++++++++++++++++++++++++ oq/module.go | 4 +-- swagger/roundtrip_test.go | 3 ++ 10 files changed, 100 insertions(+), 18 deletions(-) diff --git a/.mise-tasks/lint b/.mise-tasks/lint index 105c5897..65521a40 100755 --- a/.mise-tasks/lint +++ b/.mise-tasks/lint @@ -3,17 +3,22 @@ set -euo pipefail echo "๐Ÿ” Running linting checks..." -GOLANGCI_VERSION=$(golangci-lint --version | grep -oP 'golangci-lint has version \K[0-9.]+' || echo "unknown") +GOLANGCI_VERSION=$(golangci-lint --version | sed -nE 's/.*golangci-lint has version ([0-9.]+).*/\1/p') +GOLANGCI_VERSION=${GOLANGCI_VERSION:-unknown} echo "๐Ÿงน Running golangci-lint v${GOLANGCI_VERSION} (includes go vet)..." golangci-lint run echo "๐Ÿ›ก๏ธ Running nilaway..." -if command -v nilaway >/dev/null 2>&1; then - nilaway -test=false ./... -else +NILAWAY_BIN=$(command -v nilaway || true) +if [ -z "$NILAWAY_BIN" ]; then echo "โš ๏ธ nilaway not found, installing..." go install go.uber.org/nilaway/cmd/nilaway@8ad05f0 - nilaway -test=false ./... + GO_BIN=$(go env GOBIN) + if [ -z "$GO_BIN" ]; then + GO_BIN="$(go env GOPATH)/bin" + fi + NILAWAY_BIN="$GO_BIN/nilaway" fi +"$NILAWAY_BIN" -test=false ./... echo "โœ… All linting checks passed!" \ No newline at end of file diff --git a/cmd/update-examples/main.go b/cmd/update-examples/main.go index b5ccee03..c5abe187 100644 --- a/cmd/update-examples/main.go +++ b/cmd/update-examples/main.go @@ -236,7 +236,7 @@ func generateReadmeContent(examples []ExampleInfo) string { // Generate content in the order examples appear in the file for _, example := range examples { - content.WriteString(fmt.Sprintf("## %s\n\n", example.Title)) + fmt.Fprintf(&content, "## %s\n\n", example.Title) // Add description if available if example.Description != "" { @@ -279,5 +279,5 @@ func updateReadmeFile(filename, newContent string) error { newFileContent := before + "\n\n" + newContent + after // Write the updated content - return os.WriteFile(filename, []byte(newFileContent), 0600) + return os.WriteFile(filename, []byte(newFileContent), 0600) // #nosec G703 -- filename is a trusted repository README path from local tooling } diff --git a/cmd/update-lint-docs/main.go b/cmd/update-lint-docs/main.go index b8dfdda8..90ea2eb9 100644 --- a/cmd/update-lint-docs/main.go +++ b/cmd/update-lint-docs/main.go @@ -81,7 +81,7 @@ func generateRulesTable(docGen *linter.DocGenerator[*openapi.OpenAPI]) string { desc := strings.ReplaceAll(doc.Description, "|", "\\|") // Replace newlines with spaces desc = strings.ReplaceAll(desc, "\n", " ") - content.WriteString(fmt.Sprintf("| `%s` | %s | %s |\n", doc.ID, doc.ID, doc.DefaultSeverity, desc)) + fmt.Fprintf(&content, "| `%s` | %s | %s |\n", doc.ID, doc.ID, doc.DefaultSeverity, desc) } return content.String() @@ -114,7 +114,7 @@ func updateReadmeFile(filename, newContent string) error { newFileContent := before + "\n\n" + newContent + "\n" + after // Write the updated content - return os.WriteFile(filename, []byte(newFileContent), 0600) + return os.WriteFile(filename, []byte(newFileContent), 0600) // #nosec G703 -- filename is a trusted repository README path from local tooling } func updateRuleLinks() error { @@ -170,7 +170,7 @@ func updateRuleLinks() error { // Only write if content changed if newContent != content { - if err := os.WriteFile(filePath, []byte(newContent), 0600); err != nil { + if err := os.WriteFile(filePath, []byte(newContent), 0600); err != nil { // #nosec G703 -- filePath is constrained to local linter rule source files return fmt.Errorf("failed to write %s: %w", filePath, err) } updatedCount++ diff --git a/hashing/hashing.go b/hashing/hashing.go index ae017929..7fd99955 100644 --- a/hashing/hashing.go +++ b/hashing/hashing.go @@ -111,7 +111,7 @@ func toHashableString(v any) string { case *yaml.Node: builder.WriteString(yamlNodeToHashableString(v)) default: - builder.WriteString(fmt.Sprintf("%v", v)) + fmt.Fprintf(&builder, "%v", v) } } diff --git a/internal/testutils/utils.go b/internal/testutils/utils.go index c4c40007..acc48161 100644 --- a/internal/testutils/utils.go +++ b/internal/testutils/utils.go @@ -137,7 +137,7 @@ func DownloadFile(url, cacheEnvVar, cacheDirName string) (io.ReadCloser, error) } tempDir := filepath.Join(cacheDir, cacheDirName) - if err := os.MkdirAll(tempDir, 0o750); err != nil { + if err := os.MkdirAll(tempDir, 0o750); err != nil { // #nosec G703 -- tempDir is controlled by caller in tests return nil, err } @@ -148,7 +148,7 @@ func DownloadFile(url, cacheEnvVar, cacheDirName string) (io.ReadCloser, error) filepath := filepath.Join(tempDir, filename) // check if file exists and return it otherwise download it - r, err := os.Open(filepath) // #nosec G304 -- filepath is controlled by caller in tests + r, err := os.Open(filepath) // #nosec G304,G703 -- filepath is controlled by caller in tests if err == nil { return r, nil } @@ -169,7 +169,7 @@ func DownloadFile(url, cacheEnvVar, cacheDirName string) (io.ReadCloser, error) } // Write data to cache file - f, err := os.OpenFile(filepath, os.O_CREATE|os.O_WRONLY, 0o600) // #nosec G304 -- filepath is controlled by caller in tests + f, err := os.OpenFile(filepath, os.O_CREATE|os.O_WRONLY, 0o600) // #nosec G304,G703 -- filepath is controlled by caller in tests if err != nil { return nil, err } diff --git a/linter/format/text.go b/linter/format/text.go index 52bdf098..beee0652 100644 --- a/linter/format/text.go +++ b/linter/format/text.go @@ -91,7 +91,7 @@ func (f *TextFormatter) Format(results []error) (string, error) { if len(results) > 0 { sb.WriteString("\n") - sb.WriteString(fmt.Sprintf("โœ– %d problems (%d errors, %d warnings, %d hints)\n", len(results), errorCount, warningCount, hintCount)) + fmt.Fprintf(&sb, "โœ– %d problems (%d errors, %d warnings, %d hints)\n", len(results), errorCount, warningCount, hintCount) } return sb.String(), nil diff --git a/openapi/index.go b/openapi/index.go index f9618e28..d7a9f2a6 100644 --- a/openapi/index.go +++ b/openapi/index.go @@ -8,6 +8,7 @@ import ( "sync" "github.com/speakeasy-api/openapi/internal/interfaces" + "github.com/speakeasy-api/openapi/jsonpointer" "github.com/speakeasy-api/openapi/jsonschema/oas3" "github.com/speakeasy-api/openapi/marshaller" "github.com/speakeasy-api/openapi/pointer" @@ -863,7 +864,7 @@ func (i *Index) indexSchema(ctx context.Context, loc Locations, schema *oas3.JSO i.resolutionErrs = append(i.resolutionErrs, validation.NewValidationErrorWithDocumentLocation( validation.SeverityError, "resolution-json-schema", - err, + schemaResolutionError(schema, err), getSchemaErrorNode(schema), i.documentPathForSchema(schema), )) @@ -2588,6 +2589,26 @@ func getRefTarget(schema *oas3.JSONSchemaReferenceable) string { return info.AbsoluteReference.String() } +func schemaResolutionError(schema *oas3.JSONSchemaReferenceable, err error) error { + if schema == nil || err == nil { + return err + } + + ref := schema.GetReference() + if ref == "" { + return err + } + + if errors.Is(err, jsonpointer.ErrNotFound) { + return fmt.Errorf("reference not found: %s", ref) + } + if errors.Is(err, jsonpointer.ErrInvalidPath) { + return fmt.Errorf("invalid reference path: %s", ref) + } + + return err +} + // getSchemaErrorNode returns an appropriate YAML node for error reporting. func getSchemaErrorNode(schema *oas3.JSONSchemaReferenceable) *yaml.Node { if schema == nil { diff --git a/openapi/linter/linter_test.go b/openapi/linter/linter_test.go index 0f383326..08b50d4f 100644 --- a/openapi/linter/linter_test.go +++ b/openapi/linter/linter_test.go @@ -331,6 +331,59 @@ paths: }, resolutionErrors) } +func TestOpenAPILinter_IndexResolutionErrorsSanitizeMissingInternalSchemaReference(t *testing.T) { + t.Parallel() + ctx := t.Context() + + yamlInput := ` +openapi: 3.1.0 +info: + title: Test API + version: 1.0.0 +paths: + /users: + get: + operationId: getUsers + responses: + '200': + description: ok + content: + application/json: + schema: + $ref: '#/components/schemas/ContentList' +components: + schemas: {} +` + + config := &linter.Config{ + Extends: []string{}, + Rules: []linter.RuleEntry{ + { + ID: "semantic-path-params", + Disabled: pointer.From(false), + }, + }, + } + + lntr, err := openapiLinter.NewLinter(config) + require.NoError(t, err) + + doc, _, err := openapi.Unmarshal(ctx, strings.NewReader(yamlInput)) + require.NoError(t, err) + + output, err := lntr.Lint(ctx, linter.NewDocumentInfo(doc, "/spec/openapi.yaml"), nil, nil) + require.NoError(t, err) + + var resolutionErrors []string + for _, result := range output.Results { + resolutionErrors = append(resolutionErrors, result.Error()) + } + + assert.Equal(t, []string{ + "[16:17] error resolution-json-schema reference not found: #/components/schemas/ContentList", + }, resolutionErrors) +} + func TestOpenAPILinter_IndexCircularReferenceErrorsExposed(t *testing.T) { t.Parallel() ctx := t.Context() diff --git a/oq/module.go b/oq/module.go index 8d6fa3cc..c32c2e76 100644 --- a/oq/module.go +++ b/oq/module.go @@ -33,7 +33,7 @@ func resolveModulePath(path string, searchPaths []string) (string, error) { } if filepath.IsAbs(path) { - if _, err := os.Stat(path); err == nil { + if _, err := os.Stat(path); err == nil { // #nosec G703 -- module path is an explicit user-provided query input return path, nil } } @@ -47,7 +47,7 @@ func resolveModulePath(path string, searchPaths []string) (string, error) { for _, dir := range allPaths { full := filepath.Join(dir, path) - if _, err := os.Stat(full); err == nil { + if _, err := os.Stat(full); err == nil { // #nosec G703 -- module path is resolved from configured search paths return full, nil } } diff --git a/swagger/roundtrip_test.go b/swagger/roundtrip_test.go index f7cbc4f4..fe0c0a44 100644 --- a/swagger/roundtrip_test.go +++ b/swagger/roundtrip_test.go @@ -80,6 +80,9 @@ func TestSwagger_RoundTrip(t *testing.T) { s, validationErrs, err := swagger.Unmarshal(ctx, tee, swagger.WithSkipValidation()) require.NoError(t, err) assert.Empty(t, validationErrs) + if !strings.HasPrefix(tt.location, "testdata/") && (s.GetSwagger() == "" || s.GetInfo().GetTitle() == "") { + t.Skipf("external fixture did not return a valid Swagger document: %s", tt.location) + } outBuf := bytes.NewBuffer([]byte{})