Skip to content

fix(api): return 400 instead of 500 for hostname validation errors#343

Merged
retr0h merged 7 commits intomainfrom
fix/hostname-validation-400
Apr 2, 2026
Merged

fix(api): return 400 instead of 500 for hostname validation errors#343
retr0h merged 7 commits intomainfrom
fix/hostname-validation-400

Conversation

@retr0h
Copy link
Copy Markdown
Collaborator

@retr0h retr0h commented Apr 2, 2026

Summary

  • 37 handler endpoints across 10 domains incorrectly returned HTTP 500 for hostname validation failures — fixed to return 400
  • Added 400 response definitions to all affected OpenAPI specs and regenerated code
  • Added missing validate: tags to 25+ request body properties and query parameters across docker, user, log, and file specs
  • Added validation.AtLeastOneField() helper to reject empty update bodies (group, user, cron) — prevents silent destructive behavior (e.g., clearing group members)
  • Added missing validation.Struct() calls to log query/unit handlers and user update handler
  • Added missing HTTP wiring body validation tests for 6 endpoints (certificate update, user group create/update, user password, user update, cron update)
  • Removed redundant manual limit capping in job list handler — OpenAPI validator already handles it
  • Added "Input Validation" section to development.md documenting validation patterns
  • Cleaned up stale defense-in-depth comments

Domains fixed (500→400)

certificate, log, ntp, package, process, schedule, service, sysctl, timezone, user

Test plan

  • All 17 node API packages pass
  • Build clean, lint clean
  • Zero remaining 500JSONResponse for hostname validation paths
  • All legitimate 500 returns (job client errors) unchanged
  • Empty update bodies rejected with 400 (group, user, cron)
  • AtLeastOneField has 13 test cases
  • Log priority validation tested through HTTP wiring
  • Service create/update body validation tested through HTTP wiring

🤖 Generated with Claude Code

37 handler endpoints across 10 domains incorrectly returned HTTP 500
for hostname validation failures. These are client errors (bad input)
and should be 400. Added 400 response definitions to all affected
OpenAPI specs, regenerated code, and updated handlers and tests.

Also adds missing HTTP wiring test cases for body validation in
service create and update endpoints.

Domains fixed: certificate, log, ntp, package, process, schedule,
service, sysctl, timezone, user.

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 2, 2026

Codecov Report

❌ Patch coverage is 97.05882% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/controller/api/node/user/user_update.go 50.00% 1 Missing and 1 partial ⚠️

❌ Your patch status has failed because the patch coverage (97.05%) is below the target coverage (100.00%). You can increase the patch coverage or adjust the target coverage.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #343   +/-   ##
=======================================
  Coverage   99.91%   99.91%           
=======================================
  Files         438      438           
  Lines       21239    21268   +29     
=======================================
+ Hits        21220    21249   +29     
  Misses         11       11           
  Partials        8        8           
Files with missing lines Coverage Δ
internal/controller/api/file/file_upload.go 97.64% <ø> (ø)
internal/controller/api/job/job_list.go 100.00% <ø> (+4.08%) ⬆️
...ernal/controller/api/node/certificate/ca_delete.go 100.00% <100.00%> (ø)
...nal/controller/api/node/certificate/ca_list_get.go 100.00% <100.00%> (ø)
internal/controller/api/node/log/log_query_get.go 100.00% <100.00%> (ø)
internal/controller/api/node/log/log_source_get.go 100.00% <100.00%> (ø)
internal/controller/api/node/log/log_unit_get.go 100.00% <100.00%> (ø)
internal/controller/api/node/ntp/ntp_delete.go 100.00% <100.00%> (ø)
internal/controller/api/node/ntp/ntp_get.go 100.00% <100.00%> (ø)
...nternal/controller/api/node/package/package_get.go 100.00% <100.00%> (ø)
... and 33 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ed04edc...d15d9d1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

retr0h and others added 6 commits April 2, 2026 11:05
Add HTTP wiring tests that send invalid body payloads through the
full Echo middleware stack for 6 endpoints:
- certificate PUT update (missing object)
- user POST create group (missing name)
- user POST set password (missing password)
- schedule/cron PUT update (invalid cron expression)
- user PUT update group (defense-in-depth, all optional)
- user PUT update user (defense-in-depth, all optional)

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Add x-oapi-codegen-extra-tags with validate: tags to all request
body properties and query parameters that were missing them:
- docker: name, auto_start, working_dir
- user: uid, gid, home, shell, password, system, lock (across
  create/update user and group requests)
- log: since and priority query params with oneof constraint for
  systemd priority levels
- file: name and content_type on multipart upload (documentation)

Add validation.Struct() calls to log query and unit handlers, and
defense-in-depth call to user update handler. Add unit and HTTP
wiring tests for invalid priority on log endpoints.

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
PUT endpoints for group, user, and cron update now return 400 with
"at least one field must be provided" when the request body has no
fields set. Previously, empty bodies were silently accepted — for
group update this was destructive (cleared all members via gpasswd).

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Replace manual nil-check blocks in update handlers with
validation.AtLeastOneField(), a reflect-based helper that rejects
empty update bodies through the validation framework.

Add input validation section to development.md documenting the
validation patterns: required vs omitempty tags, AtLeastOneField for
update endpoints, and defense-in-depth for action endpoints.

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
Remove manual limit capping in job list handler — the OpenAPI spec
already validates limit via min=1,max=100 tag. Remove stale
defense-in-depth comments where validation is now meaningful
(user update with AtLeastOneField). Fix garbled comment in file
upload handler.

🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
@retr0h retr0h merged commit d16126d into main Apr 2, 2026
10 of 11 checks passed
@retr0h retr0h deleted the fix/hostname-validation-400 branch April 2, 2026 19:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant