Skip to content

fix: reject non-YAML extensions for --sqlconfig flag#747

Open
dlevy-msft-sql wants to merge 2 commits intomicrosoft:mainfrom
dlevy-msft-sql:fix/sqlconfig-extension
Open

fix: reject non-YAML extensions for --sqlconfig flag#747
dlevy-msft-sql wants to merge 2 commits intomicrosoft:mainfrom
dlevy-msft-sql:fix/sqlconfig-extension

Conversation

@dlevy-msft-sql
Copy link
Copy Markdown
Contributor

@dlevy-msft-sql dlevy-msft-sql commented Apr 30, 2026

Problem

sqlcmd --sqlconfig config.json silently creates a JSON-named file but writes YAML content, leading to confusing behavior when users reference it later.

Fixes #690, Fixes #597

Root Cause

No extension validation in config.SetFileName().

Solution

  • Validate file extension in SetFileName() before proceeding
  • Reject extensions other than .yaml or .yml (extensionless files like the default ~/.sqlcmd/sqlconfig are still allowed)
  • Report via checkErr (consistent with existing error patterns in this package)

Changes

File Change
internal/config/config.go Extension validation in SetFileName
internal/config/config_test.go Tests for rejected and accepted extensions
README.md Document custom config file extension requirement

Testing

  • go build ./cmd/modern passes
  • go test ./internal/config/... passes
  • New TestSetFileName_RejectsNonYAMLExtension verifies .json/.toml rejected and .yaml/.yml accepted

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses confusing behavior where --sqlconfig accepts non-YAML extensions (e.g., .json) but still writes YAML content, by validating the config filename extension at internal/config.SetFileName() time.

Changes:

  • Add extension validation in SetFileName() to reject non-.yaml/.yml extensions (while allowing extensionless filenames).
  • Add unit test coverage for accepted/rejected extensions.
  • Update README documentation for --sqlconfig custom configuration files.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
internal/config/config.go Enforces YAML-only extensions (and rejects other extensions) for config filenames passed into the config subsystem.
internal/config/config_test.go Adds coverage ensuring .json/.toml are rejected and .yaml/.yml are accepted.
README.md Documents the expected YAML extension requirements for custom config files via --sqlconfig.

Comment thread README.md Outdated
Comment thread internal/config/config_test.go Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@dlevy-msft-sql dlevy-msft-sql marked this pull request as ready for review May 1, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add validation/warning for unsupported config file extensions Add documentation/help about supported config types

2 participants