Skip to content

[fix](ci) Protect refreshed Codex auth updates #66349

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
shuke987 wants to merge 7 commits into
base: master
Choose a base branch
from
+46 −18
Draft

[fix](ci) Protect refreshed Codex auth updates #66349

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1fc8235
[fix](ci) Protect refreshed Codex auth updates
shuke987 Aug 1, 2026
75bba68
[fix](ci) Bound OSS auth sync retries
shuke987 Aug 1, 2026
8ba6294
[fix](ci) Align OSS retry attempt count
shuke987 Aug 1, 2026
f458632
[fix](ci) Protect temporary Codex auth copy
shuke987 Aug 1, 2026
a29b470
[fix](ci) Preserve OSS auth sync diagnostics
shuke987 Aug 2, 2026
0378270
[fix](ci) Complete Codex auth sync diagnostics
shuke987 Aug 3, 2026
c42f26b
[fix](ci) Bound Codex auth sync failure paths
shuke987 Aug 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 46 additions & 18 deletions .github/workflows/code-review-runner.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,9 @@ permissions:
jobs:
code-review:
runs-on: ubuntu-latest
# Every pre-finalization step has its own timeout. Their worst-case budget,
# including review/failure/status handling and best-effort cleanup, is 153
# minutes, leaving 12 minutes for runner setup and post-job cleanup.
timeout-minutes: 165
# Pre-finalization steps can use 153 minutes and auth sync can use 8 more,
# leaving 12 minutes for runner setup and post-job cleanup.
timeout-minutes: 173
if: >-
inputs.pr_number != '' ||
(
Expand Down Expand Up @@ -215,6 +214,8 @@ jobs:
id: auth
timeout-minutes: 5
run: |
set -o pipefail

install -m 700 -d "$RUNNER_TEMP/codex-home"
printf 'CODEX_HOME=%s\n' "$RUNNER_TEMP/codex-home" >> "$GITHUB_ENV"

Expand Down Expand Up @@ -277,6 +278,9 @@ jobs:
and (.tokens.access_token | type == "string" and length > 0)
and (.tokens.refresh_token | type == "string" and length > 0)
' "$RUNNER_TEMP/codex-home/auth.json" >/dev/null
sha256sum "$RUNNER_TEMP/codex-home/auth.json" \

@github-actions github-actions Bot Aug 3, 2026

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.

[P2] Preserve failures from the digest command

This workflow does not set shell: bash, so GitHub runs these steps as bash -e without pipefail; the pipeline status is only awk's status. If sha256sum fails to read the file, awk still exits zero and this step records an empty original hash. A later valid local/remote digest then differs from that empty baseline, so the finalizer takes the normal conflict-warning path and silently drops the refreshed credential even though OSS did not change. The two finalizer hash pipelines have the same masking problem. Enable pipefail in both affected steps (or avoid the pipelines while preserving sha256sum's exit status), and cover a digest-failure path in the mock validation.

| awk '{print $1}' \
> "$RUNNER_TEMP/codex-auth-original.sha256"

cat > "$RUNNER_TEMP/codex-home/config.toml" <<EOF
cli_auth_credentials_store = "file"
Expand Down Expand Up @@ -946,27 +950,51 @@ jobs:
OSS_ENDPOINT: oss-cn-hongkong.aliyuncs.com
OSS_CODEX_SESSION_PREFIX: oss://doris-community-ci/session

- name: Sync Codex auth back to OSS
if: ${{ always() }}
continue-on-error: true
timeout-minutes: 5
- name: Sync refreshed Codex auth back to OSS
if: ${{ always() && steps.auth.outcome == 'success' }}
timeout-minutes: 8
run: |
if [ -z "$CODEX_AUTH_OSS_OBJECT" ]; then
echo "No selected Codex auth object found; skipping OSS auth sync."
set -o pipefail

if ! jq -e '
.auth_mode == "chatgpt"
and (.tokens.access_token | type == "string" and length > 0)
and (.tokens.refresh_token | type == "string" and length > 0)
' "$CODEX_HOME/auth.json" >/dev/null; then
echo "::error::Refreshed Codex auth is invalid; refusing OSS auth sync."
exit 1
fi

original_hash="$(<"$RUNNER_TEMP/codex-auth-original.sha256")"
local_hash="$(sha256sum "$CODEX_HOME/auth.json" | awk '{print $1}')"
if [ "$local_hash" = "$original_hash" ]; then
echo "Codex auth was not refreshed; skipping OSS auth sync."
exit 0
fi

if [ ! -s "$CODEX_HOME/auth.json" ]; then
echo "No Codex auth file found; skipping OSS auth sync."
umask 077
remote_auth="$(mktemp "$RUNNER_TEMP/codex-auth-current.XXXXXX")"
trap 'rm -f "$remote_auth" "${remote_auth}.temp"' EXIT
if ! ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" \
--retry-times=3 --connect-timeout=10 --read-timeout=30 \
cp -f "$CODEX_AUTH_OSS_OBJECT" "$remote_auth"; then
echo "::error::Could not verify the current OSS auth after 3 attempts."
exit 1
fi

remote_hash="$(sha256sum "$remote_auth" | awk '{print $1}')"
if [ "$remote_hash" != "$original_hash" ]; then

@github-actions github-actions Bot Aug 3, 2026

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.

[P2] Do not treat every remote mismatch as proof that the local refresh is stale

The hashes have no generation ordering. Two supported runs can both start from O0, A can refresh to O1, and B can later refresh to O2. If A uploads O1 before B reaches this read, B sees remote_hash != original_hash here and skips O2, leaving the older (and potentially revoked) generation in OSS. In the base workflow the same finalization order uploaded O1 and then O2, so this new conflict branch introduces a lost-newest-refresh case even though the remote write happened before B's read, outside the disclosed post-read race. Please prevent concurrent use of one auth object (for example with an atomic per-object lease), or carry authoritative generation/ownership metadata; hash inequality alone cannot decide which refresh is stale.

echo "::warning::OSS auth changed during this job; skipping stale auth sync."
exit 0
fi

jq -e '
.auth_mode == "chatgpt"
and (.tokens.access_token | type == "string" and length > 0)
and (.tokens.refresh_token | type == "string" and length > 0)
' "$CODEX_HOME/auth.json" >/dev/null
ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" cp -f "$CODEX_HOME/auth.json" "$CODEX_AUTH_OSS_OBJECT"
if ! ossutil -i "$OSS_AK" -k "$OSS_SK" -e "$OSS_ENDPOINT" \
--retry-times=3 --connect-timeout=10 --read-timeout=30 \
cp -f "$CODEX_HOME/auth.json" "$CODEX_AUTH_OSS_OBJECT"; then
echo "::error::Could not persist the refreshed Codex auth after 3 attempts."
exit 1
fi
echo "Uploaded refreshed Codex auth: ${CODEX_AUTH_OSS_OBJECT##*/}"
env:
OSS_AK: ${{ secrets.OSS_AK }}
OSS_SK: ${{ secrets.OSS_SK }}
Expand Down
Loading