Skip to content

Validate private endpoint IDs before registering them - #621

Merged
sdairs merged 2 commits into
fix/612-remove-ip-allow-warnfrom
fix/611-private-endpoint-id-validation
Sep 3, 2026
Merged

Validate private endpoint IDs before registering them#621
sdairs merged 2 commits into
fix/612-remove-ip-allow-warnfrom
fix/611-private-endpoint-id-validation

Conversation

@sdairs

@sdairs sdairs commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

Fixes #611

Stacked PR: this branch is based on fix/612-remove-ip-allow-warn, not main. Review only the top commit; merge after the base PR.

What

cloud service private-endpoint create --endpoint-id (and cloud service update --add-private-endpoint-id, the non-deprecated route to the same field) accepted any string. The Cloud API does not validate the value either, so a typo exited 0 and registered a dud endpoint org-wide, which then had to be unpicked from both the service and the organization by hand.

Both add-flags now use a clap value_parser, so a malformed ID fails as a usage error (exit code 2) before any request is sent.

Why this validation and no more

The provider is not known when the flag is parsed, and the three formats are unrelated:

Provider Endpoint ID
AWS VPC endpoint ID, vpce- + 8 (legacy) or 17 lowercase hex characters
GCP numeric Private Service Connect connection ID, e.g. 102600141743718403
Azure private endpoint Resource ID (/subscriptions/.../privateEndpoints/<name>) or the legacy resourceGuid

Only provider-independent mistakes are therefore rejected:

  • empty / whitespace-only values (previously sent as id: ""),
  • values containing whitespace (two IDs squeezed into one quoted flag value),
  • any value carrying vpce- that is not exactly a well-formed AWS VPC endpoint ID — which catches vpce-bogus, truncated IDs, uppercased IDs (AWS never issues those, so they would register a dud), a pasted VPC endpoint ARN, and a pasted com.amazonaws.vpce.<region>.vpce-svc-... endpoint service name.

GCP and Azure IDs have no marker that separates a typo from a valid value, so they pass through verbatim. Azure Resource IDs (values starting with /) are exempt from the AWS check, because such a resource may itself be named vpce-....

Removal flags (--remove-private-endpoint-id, cloud org update --remove-private-endpoint) are deliberately not checked: an already-registered bogus ID must stay removable, which is exactly the cleanup path the issue describes.

Not fixed: ownership / existence

Whether the endpoint exists and belongs to the caller is not checkable client-side — the CLI has no AWS/GCP/Azure credentials — and the Cloud API accepts any string. That half of the issue is an upstream API gap; the CLI can only reject values that cannot be an endpoint ID in any provider's format.

Tests

  • Clap parse tests next to the command definitions: accepted formats for all three providers (including the legacy 8-character AWS form), rejected malformed IDs on --endpoint-id and --add-private-endpoint-id asserting ErrorKind::ValueValidation, and the deliberate escape hatch on --remove-private-endpoint-id.
  • Inline unit tests for the pure validator: every provider format accepted, plus each malformed class rejected with the offending value quoted in the message.
  • tests/cli_request_shape_test.rs: a malformed ID exits 2 with no request reaching the mock server; a valid AWS ID and both non-AWS formats are forwarded verbatim in the POST body.
  • Existing vpce-1 test fixtures updated to well-formed IDs.

cargo fmt --all, cargo clippy --workspace --all-targets -- -D warnings, and cargo test -p clickhousectl all clean.

README documents the new validation, the per-provider formats, and the unvalidated ownership gap.

🤖 Generated with Claude Code

@sdairs
sdairs force-pushed the fix/611-private-endpoint-id-validation branch from c1633e9 to 9dd3fa5 Compare August 28, 2026 14:46
@sdairs
sdairs force-pushed the fix/611-private-endpoint-id-validation branch from 9dd3fa5 to 5265266 Compare August 28, 2026 19:42
sdairs and others added 2 commits September 1, 2026 21:24
`cloud service private-endpoint create --endpoint-id` and
`cloud service update --add-private-endpoint-id` accepted any string. The
API does not check the value either, so a typo registered a dud endpoint
org-wide that had to be unpicked from both the service and the
organization.

Add a clap `value_parser` on both add-flags so a malformed ID fails as a
usage error (exit 2) before any request is sent. The provider is unknown
at parse time and the three formats are unrelated (AWS `vpce-` VPC
endpoint ID, GCP numeric PSC connection ID, Azure private endpoint
Resource ID or resourceGuid), so only provider-independent mistakes are
rejected: empty values, whitespace, and any value carrying `vpce-` that
is not exactly `vpce-` plus 8 or 17 lowercase hex characters (catching
truncated, uppercased, ARN- and service-name-pasted IDs). Azure Resource
IDs are exempt from the AWS check because such a resource may itself be
named `vpce-...`. Removal flags are deliberately unchecked so an
already-registered bogus ID stays removable.

Ownership/existence is not validated: that is not checkable client-side
and remains an upstream API gap.

Tests: clap accept/reject cases for both flags plus the remove-flag
escape hatch, unit tests for the validator across all three provider
formats, and wiremock subprocess tests asserting exit 2 with no request
issued for a malformed ID and verbatim forwarding of valid AWS/GCP/Azure
IDs.

Fixes #611

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The validator accepts both the 8- and 17-character hex forms and the README
says so, but the rejection message only named 17.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@sdairs
sdairs force-pushed the fix/611-private-endpoint-id-validation branch from 5265266 to 6113dba Compare September 1, 2026 20:36

@iskakaushik iskakaushik left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@sdairs
sdairs merged commit 0e5a434 into main Sep 3, 2026
17 checks passed
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.

cloud service private-endpoint create accepts arbitrary bogus VPC endpoint IDs

2 participants