Skip to content

fix: return 400 instead of 500 for user-caused process start failures#197

Merged
hiroTamada merged 2 commits intomainfrom
fix/process-exec-400-for-user-errors
Apr 1, 2026
Merged

fix: return 400 instead of 500 for user-caused process start failures#197
hiroTamada merged 2 commits intomainfrom
fix/process-exec-400-for-user-errors

Conversation

@hiroTamada
Copy link
Copy Markdown
Contributor

@hiroTamada hiroTamada commented Apr 1, 2026

Summary

  • /process/exec and /process/spawn were returning 500 when cmd.Start() failed due to user input (command not found, bad path, permission denied), inflating our 5xx error rate and triggering false backend alerts
  • Added isUserCmdError helper that detects exec.ErrNotFound, os.ErrNotExist, and os.ErrPermission — these map to 400 since they're client input errors
  • Genuine server-side start failures (EAGAIN, ENOMEM, etc.) still return 500

Test plan

  • TestProcessExec_CommandNotFound — verifies exec returns 400 for missing binary
  • TestProcessSpawn_CommandNotFound — verifies spawn returns 400 for missing binary
  • All existing process tests pass (go test -v -race)

Made with Cursor


Note

Low Risk
Low risk: changes only error classification/HTTP status codes for process start failures in /process/exec and /process/spawn, with added coverage to prevent regressions.

Overview
Adjusts /process/exec and /process/spawn to treat common user-caused start failures (e.g., command not found, bad path/permissions, invalid executable) as 400 Bad Request instead of 500, reducing misleading 5xx alerts.

Adds an isUserCmdError helper to centralize this detection and new tests verifying both exec and spawn return 400 for a nonexistent command.

Written by Cursor Bugbot for commit 30d1881. This will update automatically on new commits. Configure here.

Bad commands (not found, bad path, permission denied) were returning 500,
polluting backend alerts and analytics. Map these to 400 since they are
client input errors, not server failures.

Made-with: Cursor
@hiroTamada hiroTamada requested review from Sayan- and rgarcia and removed request for rgarcia April 1, 2026 16:27
@Sayan-
Copy link
Copy Markdown
Contributor

Sayan- commented Apr 1, 2026

generally lgtm ^^

Add exec.ErrDot, syscall.EISDIR, syscall.ENOEXEC, syscall.ENOTDIR.

Made-with: Cursor
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.

return errors.Is(err, exec.ErrNotFound) ||
errors.Is(err, exec.ErrDot) ||
errors.Is(err, os.ErrNotExist) ||
errors.Is(err, os.ErrPermission) ||
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

os.ErrPermission catches server-side EPERM as user error

Medium Severity

errors.Is(err, os.ErrPermission) matches both syscall.EACCES and syscall.EPERM due to Go's Errno.Is mapping. Only EACCES (file permission denied) is a user-input error. EPERM (operation not permitted) can arise from server-side issues — most notably when AsRoot or AsUser sets SysProcAttr.Credential but the server lacks CAP_SETUID/CAP_SETGID. In that case cmd.Start() fails with EPERM, which gets misclassified as a 400 instead of 500, masking a server configuration problem. Using syscall.EACCES directly instead of os.ErrPermission would avoid catching EPERM.

Additional Locations (1)
Fix in Cursor Fix in Web

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think 400 is fine for now in this case

@hiroTamada hiroTamada merged commit eb0082d into main Apr 1, 2026
5 checks passed
@hiroTamada hiroTamada deleted the fix/process-exec-400-for-user-errors branch April 1, 2026 19:07
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