Skip to content

ci: chown root-owned E2E leftovers back to runner user in Clean step#913

Merged
wenxie-amd merged 2 commits into
mainfrom
dev/wenx/ci_fix_checkout_eacces
Jul 23, 2026
Merged

ci: chown root-owned E2E leftovers back to runner user in Clean step#913
wenxie-amd merged 2 commits into
mainfrom
dev/wenx/ci_fix_checkout_eacces

Conversation

@wenxie-amd

Copy link
Copy Markdown
Collaborator

The containerized E2E steps run as root and create root-owned dirs in the mounted workspace beyond the fixed rm list (data/ incl. data/huggingface, output/, pp_simulation_result/, .git internals). The next run's actions/checkout runs as the runner user and fails with EACCES while deleting them (e.g. rmdir data/huggingface). After the existing rm/find cleanup, chown any remaining root-owned files back to the runner user so the next checkout can remove them.

The containerized E2E steps run as root and create root-owned dirs in the
mounted workspace beyond the fixed rm list (data/ incl. data/huggingface,
output/, pp_simulation_result/, .git internals). The next run's
actions/checkout runs as the runner user and fails with EACCES while
deleting them (e.g. rmdir data/huggingface). After the existing rm/find
cleanup, chown any remaining root-owned files back to the runner user so
the next checkout can remove them.

Co-authored-by: Cursor <[email protected]>
Copilot AI review requested due to automatic review settings July 23, 2026 09:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens the CI workspace cleanup by adding a post-clean “safety net” to address root-owned files and directories created by containerized E2E runs, preventing subsequent actions/checkout failures due to permission errors during workspace deletion.

Changes:

  • Expanded the Clean step’s documentation to include E2E-generated root-owned directories that can break subsequent checkouts.
  • Added a cleanup fallback that chowns any remaining root-owned workspace files back to the runner user based on the workspace directory’s UID:GID.

Comment thread .github/workflows/ci.yaml
Comment on lines +599 to +600
owner="$(stat -c '%u:%g' "${GITHUB_WORKSPACE}" 2>/dev/null)"
[ -n "$owner" ] && find "${GITHUB_WORKSPACE}" -user 0 -exec chown "$owner" {} + 2>/dev/null
The end-of-run Clean step only runs when a job actually reaches it; a runner
crash/reboot/OOM or a cancellation past the grace period can leave root-owned
files (E2E data/, output/, .pytest_cache, ...) that make the NEXT run's
actions/checkout fail with EACCES. Add a step before checkout that chowns any
root-owned files back to the runner user (as root via docker, reusing the UT
container when present, else a throwaway one), so each run self-heals
regardless of how the previous one ended.

Co-authored-by: Cursor <[email protected]>
Copilot AI review requested due to automatic review settings July 23, 2026 09:38
@wenxie-amd
wenxie-amd merged commit 9a86ba8 into main Jul 23, 2026
7 of 9 checks passed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

.github/workflows/ci.yaml:251

  • chown is executed on paths returned by find without -- and without -h. If a root-owned symlink exists under the workspace, chown will dereference it and can change ownership of the symlink target (potentially outside the workspace mount). Use chown -h (don’t dereference symlinks) and pass -- before the path list to avoid filenames being parsed as options.
          FIX="find '$CWS' -user 0 -exec chown $owner {} + 2>/dev/null; true"

.github/workflows/ci.yaml:626

  • The safety-net find ... -exec chown ... should avoid dereferencing symlinks and should pass -- so filenames beginning with - can’t be interpreted as options. Without this, a root-owned symlink under the workspace could cause ownership changes on the symlink target.
          owner="$(stat -c '%u:%g' "${GITHUB_WORKSPACE}" 2>/dev/null)"
          [ -n "$owner" ] && find "${GITHUB_WORKSPACE}" -user 0 -exec chown "$owner" {} + 2>/dev/null

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.

2 participants