Skip to content

fix(web-server): return 400 for invalid env var names instead of 500 - #36807

Closed
kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:fix/web-server-value-error-400
Closed

fix(web-server): return 400 for invalid env var names instead of 500#36807
kyssta-exe wants to merge 1 commit into
NousResearch:mainfrom
kyssta-exe:fix/web-server-value-error-400

Conversation

@kyssta-exe

Copy link
Copy Markdown
Contributor

The PUT /api/env endpoint calls save_env_value() which raises ValueError for invalid env var names (e.g. empty key, invalid characters). The catch-all except Exception returned HTTP 500, making validation errors appear as internal server errors.

Fix: catch ValueError before the generic handler and return HTTP 400 with the specific validation error message.

The PUT /api/env endpoint calls save_env_value() which raises ValueError
for invalid env var names (e.g. empty key, invalid characters). The
catch-all 'except Exception' returned HTTP 500, making validation errors
appear as internal server errors.

Fix: catch ValueError before the generic handler and return HTTP 400
with the specific validation error message.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have labels Jun 1, 2026

@mxnstrexgl mxnstrexgl left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🤖 Automated PR Review

Security Scan

  • ✓ No hardcoded secrets, injection sinks, unsafe deserialization, or dependency red flags found by this automated scan.

Code Quality

  • ✓ No blocking code-quality issues found by this automated scan.
  • ℹ️ No test file changes detected; verify existing coverage exercises this behavior.

Summary

Status: APPROVE — security findings: 0, quality suggestions: 0.

Automated review; raw diff content intentionally omitted.

@kyssta-exe

Copy link
Copy Markdown
Contributor Author

👋 tiny fix — catches ValueError from save_env_value/remove_env_value and returns HTTP 400 instead of 500. Same pattern already applied to PUT /api/env in an earlier merge. Ready for review!

@teknium1

teknium1 commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Closing as already fixed on main. The PUT /api/env ValueError -> 400 handler landed earlier via #32277. The remaining gap was the DELETE side, which is now covered by #37146 (your #36840, salvaged). Thanks!

@teknium1 teknium1 closed this Jun 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P3 Low — cosmetic, nice to have type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants