fix: harden repo maintenance checks - #869
Conversation
📝 WalkthroughWalkthroughAdds ChangesScheduled-maintenance PAT fallback, artifact retention checks, and CLAUDE_BRANCH PR creation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
test/claude-workflow-contract.test.jsOops! Something went wrong! :( ESLint: 10.5.0 ReferenceError: describe is not defined test/repo-maintenance-actions-settings.test.jsOops! Something went wrong! :( ESLint: 10.5.0 ReferenceError: describe is not defined Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2416d48a5c
ℹ️ 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".
| if command -v gh >/dev/null 2>&1; then | ||
| repo="$(gh repo view --json nameWithOwner --jq '.nameWithOwner' 2>/dev/null || true)" | ||
| if [[ -n "$repo" && "$repo" != "null" ]]; then | ||
| secrets="$(gh secret list --repo "$repo" --json name --jq '.[].name' 2>/dev/null || true)" |
There was a problem hiding this comment.
When CLAUDE_PR_GITHUB_TOKEN or CLAUDE_PAT is configured as an organization Actions secret with this repository selected, the workflow's ${{ secrets.* }} references are valid, but this check only runs gh secret list --repo and therefore only sees repository-level secrets (gh secret list treats organization secrets as a separate level). In that setup --check-scheduled-maintenance fails and tells users to add a repo secret even though scheduled maintenance would run, so the check should also account for org-level secrets or treat an absent repo secret as inconclusive.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/commands/repo-maintenance.md:
- Line 4: The argument-hint on line 4 is missing the --check-required-workflows
flag that is documented as required behavior later in the file. Add
--check-required-workflows to the argument-hint list alongside the other
--check-* flags (--check-actions-pr-settings, --check-scheduled-maintenance,
--check-artifact-retention) to keep the command hints consistent with the
documented functionality.
In @.github/workflows/scheduled-maintenance.yml:
- Around line 46-50: The actions/checkout@v6 step in the "Checkout repository"
task persists credentials to git config by default, making the PAT token
available to all subsequent workflow steps. Since the workflow already passes
authentication explicitly via GH_TOKEN environment variables in later steps, add
persist-credentials: false as a new parameter in the checkout step's with block
to prevent implicit credential availability and enforce explicit token passing
for improved security.
In `@script/lib/repo_maintenance_checks.sh`:
- Around line 58-64: The condition at line 63 using `value + 0 > 30` coerces
non-numeric values to zero, allowing invalid entries like variable references or
expressions to bypass validation. Replace the coercive numeric comparison with
an explicit check that validates the value is a literal integer before comparing
it to 30. Add validation logic to reject cases where value contains non-numeric
characters or variable syntax like dollar signs, and report an error when
retention-days is not a proper literal integer value.
In `@templates/workflows/scheduled-maintenance.yml`:
- Around line 46-50: Add the `persist-credentials: false` parameter to the
`with:` section of the `actions/checkout` action in both the
`templates/workflows/scheduled-maintenance.yml` and
`.github/workflows/scheduled-maintenance.yml` files. This prevents the
long-lived fallback PAT token from being persisted in git config where it could
be exposed to subsequent git operations like `git ls-remote` and `git push` in
the workflow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 01c0a3ab-ed0b-465c-beb8-8e94293cb711
📒 Files selected for processing (9)
.claude/commands/repo-maintenance.md.github/workflows/container-security.yml.github/workflows/scheduled-maintenance.ymlscript/README.mdscript/lib/repo_maintenance_checks.shscript/repo-maintenance.shtemplates/workflows/scheduled-maintenance.ymltest/claude-workflow-contract.test.jstest/repo-maintenance-actions-settings.test.js
| description: Comprehensive repository maintenance - run all health checks and updates | ||
| allowed-tools: Read, Bash(script/repo-maintenance.sh:*), Bash(git:*), Bash(gh:*), Bash(npm:*), Bash(pnpm:*), Bash(jq:*), Skill | ||
| argument-hint: '[--mode full|quick|check-only] [--skip CATEGORY] [--create-pr] [--check-actions-pr-settings]' | ||
| argument-hint: '[--mode full|quick|check-only] [--skip CATEGORY] [--create-pr] [--check-actions-pr-settings] [--check-scheduled-maintenance] [--check-artifact-retention]' |
There was a problem hiding this comment.
Include --check-required-workflows in argument-hint.
The hint now lists new --check-* flags but omits --check-required-workflows, which is still documented as required behavior (Line 41). Keeping the hint complete avoids inconsistent command guidance.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude/commands/repo-maintenance.md at line 4, The argument-hint on line 4
is missing the --check-required-workflows flag that is documented as required
behavior later in the file. Add --check-required-workflows to the argument-hint
list alongside the other --check-* flags (--check-actions-pr-settings,
--check-scheduled-maintenance, --check-artifact-retention) to keep the command
hints consistent with the documented functionality.
| - name: Checkout repository | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | ||
| with: | ||
| fetch-depth: 1 | ||
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN }} | ||
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/scheduled-maintenance.ymlRepository: keito4/config
Length of output: 6426
🌐 Web query:
actions/checkout v6 persist-credentials default behavior github actions
💡 Result:
For actions/checkout v6, the input persist-credentials default behavior is effectively “true” (credentials are persisted so authenticated git commands work during the job). The credentials are stored in a separate file under $RUNNER_TEMP, wired into the repo’s git config via includeIf; they are removed during post-job cleanup. To opt out, set persist-credentials: false. Details grounded in the v6 action docs: - The action documents that the auth token is persisted to enable scripts to run authenticated git commands, and that you can opt out with persist-credentials: false [1][2][3]. - The v6 documentation also states that persist-credentials now stores credentials in a separate file under $RUNNER_TEMP instead of directly in.git/config [1][2][3]. - The action’s README also notes the token is removed during post-job cleanup [1][2][3]. Important behavioral implication: - Because v6 persists credentials via git includeIf pointing at the $RUNNER_TEMP credentials file, authenticated git operations that rely on those credentials should work in the workflow after checkout, but may require compatible runner/container behavior as described by the action changes [4][5]. Answer in one line: actions/checkout@v6 persists credentials by default (persist-credentials enabled), stores them under $RUNNER_TEMP, and removes them after the job; use persist-credentials: false to disable.
Citations:
- 1: https://github.com/actions/checkout/?tab=readme-ov-file
- 2: https://github.com/actions/checkout/tree/v6.0.2?tab=readme-ov-file
- 3: https://github.com/actions/checkout
- 4: actions/checkout@v5.0.1...v6.0.0
- 5: actions/checkout@v5.0.0...v6.0.0
Add persist-credentials: false to the checkout step to prevent implicit credential availability.
The actions/checkout@v6 action persists credentials by default, making the PAT available to all subsequent steps via git config. Since the workflow passes authentication explicitly via GH_TOKEN environment variables where needed (lines 88, 119), add persist-credentials: false to the checkout step to enforce explicit token passing and limit unintended credential exposure.
Proposed hardening
- name: Checkout repository
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
with:
fetch-depth: 1
+ persist-credentials: false
token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repository | |
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | |
| with: | |
| fetch-depth: 1 | |
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN }} | |
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }} | |
| - name: Checkout repository | |
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | |
| with: | |
| fetch-depth: 1 | |
| persist-credentials: false | |
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }} |
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 46-50: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/scheduled-maintenance.yml around lines 46 - 50, The
actions/checkout@v6 step in the "Checkout repository" task persists credentials
to git config by default, making the PAT token available to all subsequent
workflow steps. Since the workflow already passes authentication explicitly via
GH_TOKEN environment variables in later steps, add persist-credentials: false as
a new parameter in the checkout step's with block to prevent implicit credential
availability and enforce explicit token passing for improved security.
Source: Linters/SAST tools
| in_upload && /^[[:space:]]*retention-days:[[:space:]]*/ { | ||
| has_retention = 1 | ||
| value = $0 | ||
| sub(/.*retention-days:[[:space:]]*/, "", value) | ||
| sub(/[[:space:]#].*/, "", value) | ||
| if (value + 0 > 30) { | ||
| printf "%s: artifact retention-days is %s (expected <= 30)\n", file, value |
There was a problem hiding this comment.
Reject non-literal retention values instead of coercing them to zero.
At Line 63, value + 0 coerces non-numeric values to 0, so entries like "90" or ${{ ... }} can slip past the <= 30 gate. Fail explicitly when retention-days is not a literal integer.
🔧 Proposed fix
in_upload && /^[[:space:]]*retention-days:[[:space:]]*/ {
has_retention = 1
value = $0
sub(/.*retention-days:[[:space:]]*/, "", value)
sub(/[[:space:]#].*/, "", value)
- if (value + 0 > 30) {
+ if (value !~ /^"?[0-9]+"?$/) {
+ printf "%s: artifact retention-days must be a literal integer <= 30 (found %s)\n", file, value
+ bad = 1
+ next
+ }
+ gsub(/"/, "", value)
+ if ((value + 0) > 30) {
printf "%s: artifact retention-days is %s (expected <= 30)\n", file, value
bad = 1
}
next
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| in_upload && /^[[:space:]]*retention-days:[[:space:]]*/ { | |
| has_retention = 1 | |
| value = $0 | |
| sub(/.*retention-days:[[:space:]]*/, "", value) | |
| sub(/[[:space:]#].*/, "", value) | |
| if (value + 0 > 30) { | |
| printf "%s: artifact retention-days is %s (expected <= 30)\n", file, value | |
| in_upload && /^[[:space:]]*retention-days:[[:space:]]*/ { | |
| has_retention = 1 | |
| value = $0 | |
| sub(/.*retention-days:[[:space:]]*/, "", value) | |
| sub(/[[:space:]#].*/, "", value) | |
| if (value !~ /^"?[0-9]+"?$/) { | |
| printf "%s: artifact retention-days must be a literal integer <= 30 (found %s)\n", file, value | |
| bad = 1 | |
| next | |
| } | |
| gsub(/"/, "", value) | |
| if ((value + 0) > 30) { | |
| printf "%s: artifact retention-days is %s (expected <= 30)\n", file, value | |
| bad = 1 | |
| } | |
| next | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@script/lib/repo_maintenance_checks.sh` around lines 58 - 64, The condition at
line 63 using `value + 0 > 30` coerces non-numeric values to zero, allowing
invalid entries like variable references or expressions to bypass validation.
Replace the coercive numeric comparison with an explicit check that validates
the value is a literal integer before comparing it to 30. Add validation logic
to reject cases where value contains non-numeric characters or variable syntax
like dollar signs, and report an error when retention-days is not a proper
literal integer value.
| - name: Checkout repository | ||
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | ||
| with: | ||
| fetch-depth: 1 | ||
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN }} | ||
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
for path in [
Path("templates/workflows/scheduled-maintenance.yml"),
Path(".github/workflows/scheduled-maintenance.yml"),
]:
if not path.exists():
continue
text = path.read_text()
idx = text.find("uses: actions/checkout")
if idx == -1:
continue
next_step = text.find("\n - name:", idx + 1)
block = text[idx: next_step if next_step != -1 else len(text)]
if "secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT" in block and "persist-credentials: false" not in block:
print(f"{path}: checkout persists the fallback PAT credentials")
PY
rg -n -C3 '\bgit\s+(push|ls-remote)\b' --glob 'script/**' --glob '.github/workflows/**' --glob 'templates/workflows/**'Repository: keito4/config
Length of output: 4147
Add persist-credentials: false to both workflow copies to prevent reusing the long-lived fallback PAT.
actions/checkout persists credentials by default; both templates/workflows/scheduled-maintenance.yml and .github/workflows/scheduled-maintenance.yml can use CLAUDE_PAT as a fallback, exposing a long-lived token to later Claude/npm/script commands that can read it from git config. The workflows contain git ls-remote and git push operations that would require explicit credentials if persistence is disabled.
Proposed hardening
- name: Checkout repository
uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6
with:
fetch-depth: 1
+ persist-credentials: false
token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Checkout repository | |
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | |
| with: | |
| fetch-depth: 1 | |
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN }} | |
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }} | |
| - name: Checkout repository | |
| uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6 | |
| with: | |
| fetch-depth: 1 | |
| persist-credentials: false | |
| token: ${{ secrets.CLAUDE_PR_GITHUB_TOKEN || secrets.CLAUDE_PAT }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/workflows/scheduled-maintenance.yml` around lines 46 - 50, Add the
`persist-credentials: false` parameter to the `with:` section of the
`actions/checkout` action in both the
`templates/workflows/scheduled-maintenance.yml` and
`.github/workflows/scheduled-maintenance.yml` files. This prevents the
long-lived fallback PAT token from being persisted in git config where it could
be exposed to subsequent git operations like `git ls-remote` and `git push` in
the workflow.
|
🎉 This PR is included in version 1.119.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
1 participant
Summary
Verification
Summary by CodeRabbit
New Features
Improvements
Documentation