Skip to content

fix: exec exit-code 127 vs 126 misclassification on 32-bit musl/seccomp#217

Merged
HiGarfield merged 1 commit into
masterfrom
copilot/fix-error-26646307016
May 29, 2026
Merged

fix: exec exit-code 127 vs 126 misclassification on 32-bit musl/seccomp#217
HiGarfield merged 1 commit into
masterfrom
copilot/fix-error-26646307016

Conversation

Copilot AI commented May 29, 2026

Copy link
Copy Markdown

On musl i486 (32-bit static) under Ubuntu 24.04, execvp returns EACCES instead of ENOENT for a non-existent path — likely because the GitHub Actions seccomp profile filters 32-bit execve syscalls differently. The old code used errno == ENOENT to distinguish exit 127 (not found) from exit 126 (not executable), so EACCES was misclassified as 126, failing assert(exit_code == 127) in test_limiter_run_command_mode_nonexistent.

Change

src/limiter.cexec_child_process

For explicit paths (argument contains /), replace the errno-based 127-vs-126 decision with access(path, F_OK):

/* Before */
if (saved_errno == ENOENT && strchr(cfg->command_args[0], '/') != NULL &&
    access(cfg->command_args[0], F_OK) == 0) {
    _exit(EXIT_CMD_NOT_EXECUTABLE);          /* 126 */
}
_exit(saved_errno == ENOENT ? EXIT_CMD_NOT_FOUND : EXIT_CMD_NOT_EXECUTABLE);

/* After */
if (strchr(cfg->command_args[0], '/') != NULL) {
    _exit(access(cfg->command_args[0], F_OK) == 0
              ? EXIT_CMD_NOT_EXECUTABLE      /* file exists → 126 */
              : EXIT_CMD_NOT_FOUND);         /* file absent  → 127 */
}
_exit(saved_errno == ENOENT ? EXIT_CMD_NOT_FOUND : EXIT_CMD_NOT_EXECUTABLE);

access(F_OK) is the authoritative answer to "does the file exist" and is unaffected by whatever errno execvp returns. PATH-resolved names (no /) still fall back to errno since access cannot resolve them against $PATH.

On musl i486 under Ubuntu 24.04, the 32-bit execve syscall may return
EACCES instead of ENOENT for a non-existent path (seccomp filtering of
32-bit syscalls).  The old code checked errno==ENOENT to decide between
exit code 127 (not found) and 126 (not executable), so it emitted 126
when EACCES was returned even though the binary does not exist, failing
the test assertion.

For explicit paths (those containing '/'), switch to using access(F_OK)
to determine 127 vs 126: if the file does not exist the command was not
found (127); if it exists it was found but could not be executed (126).
This is independent of errno and handles any EACCES/ENOENT variant.
PATH-resolved names (no '/' in argument) continue to use errno because
access() cannot evaluate PATH-relative lookups.
@HiGarfield HiGarfield marked this pull request as ready for review May 29, 2026 16:49
Copilot AI review requested due to automatic review settings May 29, 2026 16:49

Copilot AI 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.

Pull request overview

Fixes misclassification of exec failure exit codes (127 vs 126) on 32-bit musl/seccomp environments where execvp returns EACCES instead of ENOENT for non-existent paths. For explicit paths containing /, the code now uses access(F_OK) as the authoritative existence check rather than relying on errno.

Changes:

  • Replace errno-based 127-vs-126 decision with access(F_OK) for explicit paths in exec_child_process.
  • Update the surrounding comment to document the rationale (musl/seccomp EACCES behavior).

@chatgpt-codex-connector chatgpt-codex-connector Bot 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 296e9ff763

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread src/limiter.c
Comment on lines +271 to +273
_exit(access(cfg->command_args[0], F_OK) == 0
? EXIT_CMD_NOT_EXECUTABLE
: EXIT_CMD_NOT_FOUND);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve permission errors for explicit paths

When an explicit command path is inside a directory the caller cannot search, execvp() fails with EACCES and access(path, F_OK) also fails with EACCES even though the target may exist; the new ternary maps every access() failure to 127. This changes permission-denied/other lookup failures from the documented shell-compatible 126 to “command not found”; only ENOENT/ENOTDIR from the existence check should become 127, while EACCES and similar errors should remain 126.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot address this issue.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@copilot address the issue:

When an explicit command path is inside a directory the caller cannot search, execvp() fails with EACCES and access(path, F_OK) also fails with EACCES even though the target may exist; the new ternary maps every access() failure to 127. This changes permission-denied/other lookup failures from the documented shell-compatible 126 to “command not found”; only ENOENT/ENOTDIR from the existence check should become 127, while EACCES and similar errors should remain 126.

Copilot AI requested a review from HiGarfield May 29, 2026 16:56
@HiGarfield HiGarfield merged commit facddb5 into master May 29, 2026
15 checks passed
@HiGarfield HiGarfield deleted the copilot/fix-error-26646307016 branch May 29, 2026 17:13
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