Skip to content

fix(alias): report endpoint validation as usage errors#185

Merged
overtrue merged 2 commits intomainfrom
codex/test-alias-endpoint-validation
May 9, 2026
Merged

fix(alias): report endpoint validation as usage errors#185
overtrue merged 2 commits intomainfrom
codex/test-alias-endpoint-validation

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

@overtrue overtrue commented May 7, 2026

Related Issue

No linked issue. This follows recent alias endpoint validation work and adds a focused regression for a missed JSON error contract path.

Background

alias set now rejects endpoint URLs that embed credentials, but JSON-mode validation errors should still be reported as usage errors with the same exit code metadata as the process exit status. Without an explicit usage code on this path, the JSON error payload can lose the code: 2 field and be inferred from the word "credentials" instead of from the command's actual usage error result.

Solution

Emit endpoint validation failures through error_with_code(ExitCode::UsageError, ...) so JSON error metadata matches the command's returned exit code. Add an integration regression that runs alias set --json with credentials embedded in the endpoint and verifies the alias is rejected without persisting or echoing the secret.

Validation

  • cargo test -p rustfs-cli --test error_contract alias_set_json_error_rejects_embedded_endpoint_credentials -- --nocapture
  • cargo test -p rustfs-cli --test error_contract
  • make pre-commit

@overtrue overtrue marked this pull request as ready for review May 7, 2026 16:15
Copilot AI review requested due to automatic review settings May 7, 2026 16:15
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f0795f6496

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/cli/tests/error_contract.rs
Copy link
Copy Markdown
Contributor

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 pull request ensures alias set endpoint validation failures are emitted as explicit usage errors in JSON mode so the JSON error payload’s code metadata matches the actual process exit status.

Changes:

  • Emit alias endpoint validation failures via formatter.error_with_code(ExitCode::UsageError, ...) to preserve JSON code: 2.
  • Add an integration regression test ensuring alias set --json rejects endpoints with embedded credentials without leaking or persisting secrets.
  • Add a small test helper to run the CLI with an isolated config directory.

Reviewed changes

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

File Description
crates/cli/src/commands/alias.rs Reports endpoint validation failures as explicit usage errors to keep JSON error metadata aligned with exit codes.
crates/cli/tests/error_contract.rs Adds a regression test for the JSON error contract on embedded-credentials endpoints (and a config-isolated runner helper).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread crates/cli/src/commands/alias.rs
Comment thread crates/cli/tests/error_contract.rs
@overtrue overtrue merged commit 1d311c1 into main May 9, 2026
15 checks passed
@overtrue overtrue deleted the codex/test-alias-endpoint-validation branch May 9, 2026 13:17
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.

2 participants