Skip to content

cli: validate endpoint and inputs before creating resources#2789

Merged
heyitsaamir merged 5 commits intomainfrom
fix/validate-endpoint-before-creation
Apr 27, 2026
Merged

cli: validate endpoint and inputs before creating resources#2789
heyitsaamir merged 5 commits intomainfrom
fix/validate-endpoint-before-creation

Conversation

@heyitsaamir
Copy link
Copy Markdown
Collaborator

@heyitsaamir heyitsaamir commented Apr 25, 2026

Summary

  • Adds validateEndpoint() to catch bad URLs (placeholders, HTTP, malformed) before any resources get created
  • Moves all field validations (name length, description length, URL format) upfront in app update so a bad flag doesn't leave the app half-updated
  • Adds inline validate on interactive endpoint prompts in both create and update so users get immediate feedback

Test plan

  • teams app create --name Test --endpoint "https://<foo>/api" --yes → fails immediately with "Endpoint is not a valid URL."
  • teams app create --name Test --endpoint "http://example.com/api" --yes → fails with "Endpoint must use HTTPS."
  • teams app update <id> --endpoint "ftp://bad" --json → fails with validation error before any API calls
  • teams app update <id> --name "x]x31" --json → fails upfront, no partial mutations
  • Interactive app create → entering a bad endpoint re-prompts instead of accepting
  • npm run test passes

Bad endpoint URLs (placeholders, HTTP, malformed) now fail immediately
instead of after AAD app + secret creation. Adds shared validateEndpoint()
and moves all field validations upfront in both create and update commands.

Co-Authored-By: Claude <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 25, 2026 16:00
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

Adds centralized endpoint validation and moves CLI flag validation earlier to prevent partial updates/resource creation when user inputs are invalid.

Changes:

  • Introduce validateEndpoint() (HTTPS + placeholder checks) and wire it into app create / app update.
  • Move app update field validations (name/description/URLs/endpoint) ahead of token/API work.
  • Add inline validation to interactive endpoint prompts so invalid endpoints are rejected immediately.

Reviewed changes

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

File Description
packages/cli/src/commands/app/update.ts Adds upfront validation (incl. endpoint) and prompt-time endpoint validation.
packages/cli/src/commands/app/create.ts Validates endpoint flag early and adds prompt-time validation for endpoint input.
packages/cli/src/apps/manifest.ts Adds shared validateEndpoint() helper used by commands.

Comment thread packages/cli/src/commands/app/create.ts Outdated
Comment thread packages/cli/src/commands/app/create.ts
Comment thread packages/cli/src/commands/app/update.ts Outdated
Comment thread packages/cli/src/commands/app/update.ts Outdated
Comment thread packages/cli/src/commands/app/create.ts Outdated
Comment thread packages/cli/src/apps/manifest.ts
heyitsaamir and others added 2 commits April 25, 2026 09:23
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Use `!== undefined` + trim for endpoint in update.ts (matches create.ts)
- Add empty hostname guard in validateEndpoint()
- Remove double icon read in create.ts by reusing early validation result
- Add 12 unit tests for validateEndpoint and CLI endpoint validation

Co-Authored-By: Claude <noreply@anthropic.com>
app create checks auth before validation, so those CLI tests fail
in CI (no login). Keep unit tests + app update --json tests which
hit validation before auth.

Co-Authored-By: Claude <noreply@anthropic.com>
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

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

Comment thread packages/cli/src/commands/app/create.ts
Comment thread packages/cli/src/apps/manifest.ts Outdated
- Validation now runs before getAccount() so bad inputs fail even
  without login (fixes CI test issue)
- Remove unreachable placeholder <> check — URL parser already
  rejects those characters

Co-Authored-By: Claude <noreply@anthropic.com>
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

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Comment thread packages/cli/src/commands/app/update.ts
@heyitsaamir
Copy link
Copy Markdown
Collaborator Author

Re: httpsUrlRegex not enforcing hostname — good catch in principle, but https:///path already throws in Node's URL parser so the regex can't actually accept it. The regex is pre-existing code not introduced by this PR — happy to tighten it in a follow-up but keeping out of scope here.

@heyitsaamir heyitsaamir merged commit f4f04b6 into main Apr 27, 2026
7 checks passed
@heyitsaamir heyitsaamir deleted the fix/validate-endpoint-before-creation branch April 27, 2026 13:03
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.

3 participants