Skip to content

[Fix] Deployment commands reject registries with explicit ports - #2313

Open
roomote-roomote[bot] wants to merge 2 commits into
developfrom
fix/registry-port-validation-0ovminoanlj9c
Open

[Fix] Deployment commands reject registries with explicit ports#2313
roomote-roomote[bot] wants to merge 2 commits into
developfrom
fix/registry-port-validation-0ovminoanlj9c

Conversation

@roomote-roomote

@roomote-roomote roomote-roomote Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

​Opened on behalf of @daniel-lxs. Follow up by mentioning @roomote-roomote, in the web UI, or in Slack.

Related issue

No linked issue. This is a maintainer-authorized operator CLI correction.

Why this PR exists

  • A maintainer explicitly invited this PR in the linked issue or discussion
  • I am a maintainer / this is internal Roomote work

The remote upgrade and deploy commands reject valid Docker registries such as registry.example:5000 before reaching SSH. They currently apply the namespace character allowlist to registry addresses, which excludes the port separator.

What changed

Allow a numeric port in the range 1-65535 on the registry host in both commands while keeping the existing portless validation, namespace allowlist, and path-character restrictions unchanged. Operators can now pass --image-registry registry.example:5000 or localhost:5000, including registry paths. Zero, out-of-range and oversized numeric ports, colons in namespaces, malformed ports, URL schemes, whitespace, and shell syntax are rejected. Leading zeros retain decimal meaning, and significant digits are bounded before arithmetic to prevent overflow. This is not a full Docker image-reference grammar rewrite or an IPv6 support change.

Add credential-free regression coverage through the actual Bash CLIs and run it as part of the existing deployment-artifact CI check. No remote execution or deployment lifecycle logic changes.

How it was tested

Base: 2861493ab271b9040e11959965b9c3721ebd9a9c. Head: ae024ce4748067b8ec26cf207cc0b1af92d7b53f.

  • The original regression failed on registry.example:5000; the range follow-up failed on registry.example:0 before tightening. bash deploy/scripts/tests/image-registry.sh now passes all 64 cases across deploy and upgrade, explicitly accepting 1 and 65535 and rejecting 0, 65536, and a 30-digit value. Coverage retains portless registries, host:port/path, localhost, nested namespaces, leading-zero decimal boundaries, malformed ports, and unsafe inputs. Tests deliberately set retention to zero so accepted registry inputs stop at the next validation gate before SSH, Terraform, or deployment state writes.
  • bash deploy/host/tests/upgrade-failed-pull.sh passed, preserving deployment metadata on the mocked failed-pull path.
  • bash -n on the changed shell scripts and git diff --check passed.
  • pnpm lint, pnpm check-types (27 packages), and pnpm knip passed. Unmodified pre-push hooks also passed oxlint, residual ESLint, fast typechecks, and Knip. Knip reports a non-failing existing docs mint configuration hint.
  • pnpm deployment:validate passed the CLI regressions and all six Compose shapes, then could not complete Caddy adaptation because /var/run/docker.sock is absent in the sandbox. CI must confirm the complete deployment-artifact gate.
  • No live SSH, registry pull, deployment, or full monorepo test run was performed. Browser proof is not applicable to this operator argument-validation change.

Checklist

  • The PR title follows the repo convention: [Fix], [Feat], [Improve], [Refactor], [Docs], or [Chore] followed by a user-facing description
  • This PR is small and scoped to one change
  • pnpm lint and pnpm check-types pass locally
  • I added tests or included a clear manual validation note above
  • I removed secrets, tokens, private keys, and customer data from code, logs, and screenshots
  • If this change should appear in the changelog, I ran pnpm changeset

No changeset added for this bounded operator CLI correction.

@roomote-community

roomote-community Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

No code issues found. See task

Reviewed ae024ce

@roomote-roomote
roomote-roomote Bot marked this pull request as ready for review September 7, 2026 05:40
@roomote-roomote

roomote-roomote Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Fixed in ae024ce4. Registry ports now accept only numeric values 1–65535, including decimal leading zeros, with overflow-safe handling. Existing portless registries, host:port/path, namespace restrictions, and unsafe-input rejection are preserved.

All 64 real-CLI regressions pass across deploy and upgrade, including accepted 1/65535 and rejected 0/65536/oversized values; the new port-zero test failed before tightening. Local lint, types, Bash syntax, adjacent failed-pull regression, and intact pre-push gates pass. All 15 non-skipped checks on the exact head pass, including deployment artifacts, Test, upgrade compatibility, Docker build, and automated review; two optional/matrix checks skipped. CI completed Docker-dependent deployment validation that could not finish locally because the sandbox lacks its Docker socket.

No live registry pull or SSH deployment was performed. Browser proof is not applicable. Existing PR state was preserved; no new PR or merge.

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.

1 participant