Skip to content

fix: assorted small validation and robustness fixes#2410

Closed
andrewwhitecdw wants to merge 3 commits into
NVIDIA:mainfrom
andrewwhitecdw:fix-assorted-validation-robustness/aw
Closed

fix: assorted small validation and robustness fixes#2410
andrewwhitecdw wants to merge 3 commits into
NVIDIA:mainfrom
andrewwhitecdw:fix-assorted-validation-robustness/aw

Conversation

@andrewwhitecdw

Copy link
Copy Markdown

Summary

Three small, independent robustness fixes found during code review, one commit each:

  1. server: Kubernetes label key/value validation used Unicode-aware is_alphanumeric(), so labels like café or 日本語 passed gateway validation but would be rejected by the Kubernetes API server (the label spec is ASCII-only). The same functions already validate the key prefix with ASCII-only checks.
  2. cli: two display paths in sandbox policy commands sliced server-supplied strings unguarded — &resp.policy_hash[..12] (panics on empty/short proto3 hash; a guarded short_hash() helper already existed nearby) and &rev.load_error[..40] (panics on multi-byte UTF-8, same bug class as fix(cli): avoid panic on multi-byte UTF-8 in --since duration #2406).
  3. sandbox: the ephemeral-port advisory check used port > 49152, omitting port 49152 itself from the IANA dynamic/private range (49152–65535 inclusive).

Related Issue

N/A — small fixes found during code review.

Changes

  • validate_label_key/validate_label_value: use is_ascii_alphanumeric(); added Unicode rejection tests
  • sandbox_policy_set: use existing short_hash() helper instead of unguarded slicing
  • Policy revision table: back off to a char boundary when truncating load_error
  • mechanistic_mapper: port >= 49152 for the ephemeral range note

Testing

  • mise run pre-commit passes (mise unavailable in this environment; ran equivalent cargo fmt + cargo clippy on all touched crates — clean)
  • Unit tests added/updated (cargo test -p openshell-server validate_label — 40 passed, incl. 2 new)
  • E2E tests added/updated (if applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable)

validate_label_key and validate_label_value used Unicode-aware
char::is_alphanumeric(), so values like 'café' or '日本語' passed
gateway validation even though the Kubernetes label spec
(([A-Za-z0-9][-A-Za-z0-9_.]*)?[A-Za-z0-9])? is ASCII-only and the
API server rejects such labels later. The same functions already
validate the key prefix with ASCII-only checks.

Use is_ascii_alphanumeric() and add regression tests for Unicode
keys and values.

Signed-off-by: Andrew White <andrewh@cdw.com>
Two display paths in sandbox policy commands sliced server-supplied
strings without guarding:

- sandbox_policy_set used &resp.policy_hash[..12] unconditionally;
  an empty or short proto3 hash field would panic. Use the existing
  short_hash() helper, as nearby call sites already do.
- The policy revision table truncated load_error at a fixed byte
  index (&rev.load_error[..40]), panicking on multi-byte UTF-8 in
  server error messages. Back off to a char boundary instead.

Signed-off-by: Andrew White <andrewh@cdw.com>
The IANA dynamic/private (ephemeral) port range is 49152-65535
inclusive, but the check used port > 49152, silently omitting the
advisory note for port 49152 itself.

Signed-off-by: Andrew White <andrewh@cdw.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@github-actions

Copy link
Copy Markdown

Thank you for your interest in contributing to OpenShell, @andrewwhitecdw.

This project uses a vouch system for first-time contributors. Before submitting a pull request, you need to be vouched by a maintainer.

To get vouched:

  1. Open a Vouch Request discussion.
  2. Describe what you want to change and why.
  3. Write in your own words — do not have an AI generate the request.
  4. A maintainer will comment /vouch if approved.
  5. Once vouched, open a new PR (preferred) or reopen this one after a few minutes.

See CONTRIBUTING.md for details.

@github-actions

Copy link
Copy Markdown

Thank you for your submission! We ask that you sign our Developer Certificate of Origin before we can accept your contribution. You can sign the DCO by adding a comment below using this text:


I have read the DCO document and I hereby sign the DCO.


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the DCO Assistant Lite bot.

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