Skip to content

the 'can_push' check is failing, even on repos with write access#149

Merged
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:the-canpush-check-is-failing-even-on-repos-wit
Apr 23, 2026
Merged

the 'can_push' check is failing, even on repos with write access#149
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:the-canpush-check-is-failing-even-on-repos-wit

Conversation

@derekmisler
Copy link
Copy Markdown
Contributor

@derekmisler derekmisler commented Apr 23, 2026

Related Issues

Closes: https://github.com/docker/gordon/issues/438

Summary

Removes a faulty can_push permission check in the release workflow that was incorrectly failing even when the action had write access to repositories. The check was overly restrictive and prevented legitimate push operations, so it's being removed to allow the workflow to proceed with update attempts.

Changes

  • .github/workflows/release.yml: Removed the gh api call that checked .permissions.push and the associated skip logic (lines 477-485). This check was causing false negatives when the app actually had write access.

Signed-off-by: Derek Misler <derek.misler@docker.com>
@derekmisler derekmisler self-assigned this Apr 23, 2026
@derekmisler derekmisler requested a review from a team April 23, 2026 18:07
@derekmisler derekmisler marked this pull request as ready for review April 23, 2026 18:07
@derekmisler derekmisler enabled auto-merge (squash) April 23, 2026 18:07
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🟢 APPROVE

The removal of the can_push pre-flight check is safe. Here's what the analysis found:

No hard failures or data-loss risks introduced. The downstream signed-commit call at line ~505 already has a || { echo "::warning::…"; cd /; rm -rf "$WORK_DIR"; continue; } error handler that catches push failures gracefully and continues the loop — so the workflow will not crash or corrupt state when the app lacks write access to a repo.

Existing guards remain intact:

  • The gh repo clone check (line ~469) still skips repos where the token cannot even clone
  • The file-exists and pattern-match checks still skip repos where the file/pattern isn't found
  • The signed-commit error handler still skips repos where the push fails

Net behavioural change (low severity, as-designed): Instead of a cheap skip before any I/O, the workflow now performs a full clone → sed → API call before discovering it lacks write access. This produces slightly noisier warning output and wastes some runner time per inaccessible repo — but this is the accepted trade-off described in the PR description to fix the false-negative permission check.

No CONFIRMED or LIKELY bugs were found in the changed lines.

@derekmisler derekmisler merged commit 558e434 into docker:main Apr 23, 2026
19 checks passed
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