Skip to content

Strip GitHub auth from runner job environment#142

Merged
jmcte merged 4 commits into
mainfrom
codex/fix-vulnerability-in-public-runner-allowlist-bc7j9i
Jun 24, 2026
Merged

Strip GitHub auth from runner job environment#142
jmcte merged 4 commits into
mainfrom
codex/fix-vulnerability-in-public-runner-allowlist-bc7j9i

Conversation

@jmcte

@jmcte jmcte commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Motivation

  • Public/selected Synology runner containers previously inherit long-lived GitHub credentials (GITHUB_PAT and GitHub App envs) which job-level processes can read, creating a credential-exposure vulnerability.

Description

  • Add a new sanitized job launch helper run_runner_job_bash in docker/runner-entrypoint.sh that launches ./run.sh with env -u to unset GITHUB_PAT, GITHUB_APP_ID, GITHUB_APP_INSTALLATION_ID, and GITHUB_APP_PRIVATE_KEY before executing the job process.
  • Switch the runner job start path to use the sanitized run_runner_job_bash so registration/cleanup still use the existing credential-bearing path while jobs run without inherited GitHub auth.
  • Extend the smoke runner stub scripts/smoke/actions-runner/run.sh to fail if any GitHub auth env vars are present and to record the sanitized state, and add an assertion to scripts/smoke-test.sh to verify the job process sees github auth: unset.
  • Key files changed: docker/runner-entrypoint.sh, scripts/smoke/actions-runner/run.sh, and scripts/smoke-test.sh.

Testing

  • Ran the TypeScript build and unit tests with pnpm --silent build && pnpm --silent test, and all tests passed (48 test files, 263 tests succeeded).
  • Performed a shell syntax check bash -n on modified shell scripts which succeeded.
  • Executed the smoke harness pnpm --silent smoke-test but it was blocked in this environment because Docker is not installed (docker: command not found), so full smoke execution could not complete here.

Codex Task

@jmcte jmcte enabled auto-merge (squash) June 22, 2026 08:50

@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: a1e6a26ca9

ℹ️ 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 docker/runner-entrypoint.sh
athena-omt
athena-omt previously approved these changes Jun 22, 2026

@athena-omt athena-omt 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.

Athena review: approves. The change is focused, tests cover the new guard, and I don't see a blocker in the diff.

pheidon
pheidon previously approved these changes Jun 22, 2026

@pheidon pheidon 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.

Pheidon review: approves. The change is focused, tests cover the new guard, and I don't see a blocker in the diff.

@pheidon pheidon dismissed stale reviews from athena-omt and themself via 4e36cff June 22, 2026 23:15
@pheidon pheidon force-pushed the codex/fix-vulnerability-in-public-runner-allowlist-bc7j9i branch 2 times, most recently from 4e36cff to 9ecf728 Compare June 22, 2026 23:19
@pheidon pheidon force-pushed the codex/fix-vulnerability-in-public-runner-allowlist-bc7j9i branch from 9ecf728 to 7fc56cb Compare June 22, 2026 23:24
athena-omt
athena-omt previously approved these changes Jun 23, 2026

@athena-omt athena-omt 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.

I checked the PR branch in the actual worktree and the fix looks complete. The runner entrypoint now stashes GitHub auth only for cleanup, unsets it before launching the job process, and the smoke harness asserts the job sees GitHub auth as unset. I also sanity-checked the CI switch to ./actions/setup-shell-safe-node; it matches the repository’s wrapper and pinned Node version.

No new blocker found.

# Conflicts:
#	docker/runner-entrypoint.sh
#	scripts/smoke/actions-runner/run.sh
#	test/smoke-harness.test.ts
#	test/workflow.test.ts

@athena-omt athena-omt 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.

APPROVE

I synced the PR mirror and reviewed the live diff in the worker worktree. The runner entrypoint now strips GitHub auth before launching the job process while preserving the credential-bearing cleanup path, and the smoke harness plus workflow tests line up with that behavior. I did not find a new merge blocker beyond the existing queue state.

@jmcte jmcte merged commit f601403 into main Jun 24, 2026
19 checks passed
@jmcte jmcte deleted the codex/fix-vulnerability-in-public-runner-allowlist-bc7j9i branch June 24, 2026 21:16
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.

4 participants