Skip to content

fix(forms): validate Helm release name, ignored namespace, and port-forward port#573

Merged
nadaverell merged 6 commits intomainfrom
feat/fix-helm-audit-portforward-input-validation
May 2, 2026
Merged

fix(forms): validate Helm release name, ignored namespace, and port-forward port#573
nadaverell merged 6 commits intomainfrom
feat/fix-helm-audit-portforward-input-validation

Conversation

@eliran-ops
Copy link
Copy Markdown
Contributor

@eliran-ops eliran-ops commented Apr 29, 2026

Summary

Adds shared input validation to three Radar forms that were previously accepting bad input silently. Linear: SKY-821.

Bugs fixed:

  • 24 — Helm install accepted release names like `Invalid Name With Spaces!` and proceeded to step 2; the install only failed server-side.
  • 25 — Audit Settings → Add Ignored Namespace accepted `INVALID_NAME!@#` with no validation.
  • 26 — Port-Forward dialog silently swallowed `0`, `-1`, `70000` and similar in the local-port input.
  • 27 — No rejection feedback at all in the port-forward dialog.
  • 63 — Audit Settings 'Save' button stayed enabled when the only pending edit was an invalid namespace string.

Approach

New module `packages/k8s-ui/src/utils/validators.ts` with pure validators:

Function Used for Rule
`validateRFC1123Label` k8s namespace / pod / service names `a-z0-9?`, ≤63 chars
`validateRFC1123Subdomain` dotted DNS subdomains labels joined by '.', ≤253 chars
`validateHelmReleaseName` Helm release names RFC 1123 subdomain + Helm's 53-char cap
`validatePort` TCP/UDP ports as text strict integer in [1, 65535], no decimals/scientific/+/whitespace

Each returns a discriminated-union `ValidationResult` so callers can pattern-match without re-parsing. 29 unit tests pin the canonical rules and cover every reported fixture verbatim plus the relevant boundaries.

Wiring

  • `InstallWizard.tsx` — memoized validations on release name + namespace; red border + inline red error on invalid; `canProceedFromInfo` requires both validations to pass.
  • `AuditSettingsDialog.tsx` — validates the staged input before `addNamespace`; Add (+) disabled on invalid OR duplicate input; Save disabled while there's pending invalid input with a tooltip explaining why.
  • `PortForwardButton.tsx` — tracks raw input separately from validated port so the user always sees what they typed (no silent swallow); red border + inline error on rejection; falls back to the original remote port so the kubectl command is always runnable.

Test plan

  • `cd packages/k8s-ui && npm test` — 91 passed (29 new)
  • `cd web && npm run tsc` — clean
  • `cd web && npm run build` — clean

Made with Cursor


Note

Medium Risk
Medium risk because it changes client-side validation gates for Helm installs and audit settings; overly-strict rules could block previously accepted (but possibly valid) user input and alter submission behavior.

Overview
Introduces a new shared utils/validators module (with unit tests) that codifies RFC1123 label/subdomain rules, Helm release-name constraints, and strict port parsing.

Updates the Helm InstallWizard to validate release name and namespace before advancing/installing, surface inline errors, and submit trimmed values to the API.

Updates AuditSettingsDialog to validate/deny invalid or duplicate ignored-namespace entries, show inline error text, and disable Save while there’s pending invalid input.

Updates the port-forward kubectl dialog to validate the local port as raw text (no silent truncation), show error feedback, and fall back to the original port when invalid.

Reviewed by Cursor Bugbot for commit 7426ab3. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread web/src/components/helm/InstallWizard.tsx
Comment thread packages/k8s-ui/src/utils/validators.ts Outdated
Comment thread packages/k8s-ui/src/utils/validators.ts
eliran-ops pushed a commit that referenced this pull request Apr 30, 2026
Bugbot follow-ups on PR #573.

1. validateHelmReleaseName length error trailed with a period.
   The ValidationResult contract is 'no trailing period — call
   sites compose them into sentences', and InstallWizard does
   exactly that ('Release name {error}.'), producing '...limit..'
   in the UI when a user typed > 53 chars.

2. validateRFC1123Subdomain only checked total length (≤ 253) and
   the 'segments' regex, so a single 200-char label passed
   client-side and was rejected only server-side. K8s'
   IsDNS1123Subdomain enforces ≤ 63 PER label on top of the 253
   total. Add the per-label check and pin it under 'each
   dot-separated label must be at most 63 characters'.

Made-with: Cursor
Comment thread packages/k8s-ui/src/utils/validators.ts Outdated
@eliran-ops
Copy link
Copy Markdown
Contributor Author

@nadaverell @hisco — ready for review. Cursor bugbot found 4 issues across the helm-name validators: trim mismatch in InstallWizard, trailing period in error string, missing per-label 63-char check, and validator allowing dots in release names. First three were addressed earlier (c8e583a, a668623). Just pushed e9f2586 to fix the last one — validateHelmReleaseName now delegates to validateRFC1123Label so dots are rejected up front (Helm permits them, but generated resource names <release>-<chart>-<hash> must be valid DNS-1123 labels and K8s rejects the apply server-side). Added a regression test. Cursor Bugbot SUCCESS on current HEAD.

eliran-mic and others added 6 commits May 3, 2026 00:00
…orward port

Five reported form-validation bugs (SKY-821) addressed by introducing a
shared, pure validators module and wiring it into the three offending
dialogs:

24. Helm install dialog accepted release names like "Invalid Name With
    Spaces!" and proceeded to step 2; the install only failed
    server-side, leaving users with an opaque 422.
25. Audit Settings → Add Ignored Namespace accepted "INVALID_NAME!@#"
    with no validation; the audit pipeline then silently never matched
    the bogus entry, so the user thought the ignore filter "didn't
    work".
26. Port-Forward dialog: the local-port `<input type="number">`
    swallowed values like "0", "-1", and "70000" with no feedback —
    the typed value disappeared or truncated mid-typing.
27. The port input gave no rejection feedback at all; rejected values
    just vanished.
63. Audit Settings 'Save' button stayed enabled when the only pending
    edit was an invalid namespace string (would silently drop the user's
    entry on save).

New module `packages/k8s-ui/src/utils/validators.ts` with:

- `validateRFC1123Label`     — k8s namespace / pod / service names
- `validateRFC1123Subdomain` — dotted DNS subdomains
- `validateHelmReleaseName`  — Helm's 53-char limit on top of subdomain
- `validatePort`             — strict integer in [1, 65535]; rejects
                               "0", "-1", "1.0", "1e3", "+1", "70000",
                               whitespace-padded values, anything with
                               non-digit characters

29 unit tests pin the canonical k8s / Helm rules and cover every
fixture from SKY-821 verbatim, plus boundaries (63-char label limit,
53-char Helm limit) and adversarial inputs (decimals, scientific,
truncation).

Wiring:

- `InstallWizard.tsx`: validates release name + namespace via memoized
  validators; inline red border + error text under each input;
  `canProceedFromInfo` now requires both validations to pass; preserves
  existing helper text when input is valid.
- `AuditSettingsDialog.tsx`: validates the staged-but-unsubmitted
  namespace input; disables the Add button on invalid or duplicate
  input; shows an inline red error; disables Save when there's pending
  invalid input (with a tooltip explaining why).
- `PortForwardButton.tsx`: tracks the raw input separately from the
  validated port so the user always sees the characters they typed
  (no more silent swallow); shows a red border + inline error when the
  input is rejected; falls back to the original remote port for the
  built kubectl command so the dialog always shows a runnable command.

Linear: SKY-821
Made-with: Cursor
…ated

Cursor Bugbot caught: the form validates `releaseName.trim()` and
`namespace.trim()` but `handleInstall` previously sent the raw
untrimmed values to the server. A user typing "my-release " (trailing
space) would pass green client-side validation and then get a
confusing server-side rejection — the exact class of bug this PR
exists to prevent.

Trim once, locally, before the submit call (and reuse the trimmed
strings in the success callback's namespace/release-name arguments).

Made-with: Cursor
Bugbot follow-ups on PR #573.

1. validateHelmReleaseName length error trailed with a period.
   The ValidationResult contract is 'no trailing period — call
   sites compose them into sentences', and InstallWizard does
   exactly that ('Release name {error}.'), producing '...limit..'
   in the UI when a user typed > 53 chars.

2. validateRFC1123Subdomain only checked total length (≤ 253) and
   the 'segments' regex, so a single 200-char label passed
   client-side and was rejected only server-side. K8s'
   IsDNS1123Subdomain enforces ≤ 63 PER label on top of the 253
   total. Add the per-label check and pin it under 'each
   dot-separated label must be at most 63 characters'.

Made-with: Cursor
Helm itself permits dots in release names but generated resource
names like `<release>-<chart>-<hash>` must be valid DNS-1123
labels (no dots). A dotted release like `my.app` passes Helm's
validation, then K8s rejects the resource apply server-side
with an opaque 422.

Switch validateHelmReleaseName to delegate to validateRFC1123Label
so the dot is caught with clear inline feedback before submit.

Cursor bugbot finding (validators.ts:126).
The audit dialog showed a red 'already in the list' error for duplicate
namespace input but didn't disable Save — the user could click Save and
silently lose the pending entry. Save now blocks on newNsDuplicate as well.

Also strips SKY-821 / 'bug NN' / Bugbot references from validators,
test descriptions, and the port-forward + audit comment blocks (commit
362a6bf only cleaned InstallWizard.tsx).
@nadaverell nadaverell force-pushed the feat/fix-helm-audit-portforward-input-validation branch from e9f2586 to 7426ab3 Compare May 2, 2026 21:20
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 7426ab3. Configure here.

// Reject anything other than digits — '+1', '1.0', '1e3', ' 80 '
// all fail here. Catching them as parse failures gives clearer
// feedback than silently truncating via Number().
if (!/^\d+$/.test(raw)) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Port validator allows leading zeros despite documented contract

Low Severity

The validatePort docstring on line 140 explicitly states it accepts strings with "no leading zeros," but the regex /^\d+$/ permits inputs like "01", "007", or "0080". For example, validatePort("01") returns { valid: true, value: 1 } instead of rejecting it. The test suite also has no coverage for leading-zero inputs. Either the regex needs tightening (e.g., /^([1-9]\d*|0)$/ and then range-reject 0) or the docstring needs correction, since right now callers cannot rely on the documented contract.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 7426ab3. Configure here.

@nadaverell nadaverell merged commit 20c1fe1 into main May 2, 2026
9 checks passed
@nadaverell nadaverell deleted the feat/fix-helm-audit-portforward-input-validation branch May 2, 2026 21:40
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