Skip to content

ci(manfest) updates the commit-latest script and action for the updated repo structure #3304

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

Closed
mtchoum1 wants to merge 1 commit into from
Closed

ci(manfest) updates the commit-latest script and action for the updated repo structure #3304

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 10 additions & 2 deletions .github/workflows/update-commit-latest-env.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ name: Update commit-latest.env on params-latest.env change
- main
- rhoai-*
paths:
- 'manifests/base/params-latest.env'
- 'manifests/odh/base/params-latest.env'

jobs:
sync-commit:
Expand All @@ -27,10 +27,18 @@ jobs:
token: ${{ secrets.GITHUB_TOKEN }}
persist-credentials: true
Copy link
Copy Markdown
Member

@jiridanek jiridanek Apr 9, 2026

Choose a reason for hiding this comment

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

@coderabbitai it's probably a bit of a security theater, but this can be written without having to use persist-credentials: true. See #3202

Up to you if this is worth implementing. The credentials are still in memory and malicious code, if it ever gets to run in our action, can introspect runner's memory. So maybe it's useless.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

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

🧩 Analysis chain

🏁 Script executed:

gh pr view 3202 --repo opendatahub-io/notebooks --json body,files | jq -r '.body, "\n---FILES---", (.files[].path)'

Repository: opendatahub-io/notebooks
Length of output: 2348

🏁 Script executed:

gh pr diff 3202 --repo opendatahub-io/notebooks | grep -A 30 -B 5 "persist-credentials\|x-access-token\|push\|token"

Repository: opendatahub-io/notebooks
Length of output: 2405

@jiridanek — you're right that it's a limited mitigation, but the concrete benefit is that the token is never written to .git/config on disk, so code executing in the workspace (e.g. a compromised build tool, a malicious dep pulled during uv run) cannot read it via a simple file read. Memory introspection is a higher bar. Worth doing for consistency with the pattern established in #3202.

The change is small — apply the same x-access-token push pattern:

      - name: Checkout repository
        uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd  # v6.0.2
        with:
          persist-credentials: false   # token never written to .git/config
          # no `token:` here — supplied only at push time below
      - name: Commit and push changes
        run: |
          git config --global user.name 'github-actions[bot]'
          git config --global user.email 'github-actions[bot]@users.noreply.github.com'

          changed=0
          for f in \
            manifests/odh/base/commit-latest.env \
            manifests/odh/base/commit.env \
            manifests/rhoai/base/commit-latest.env \
            manifests/rhoai/base/commit.env; do
            if ! git diff --quiet -- "$f" 2>/dev/null; then
              changed=1
              git add -- "$f"
            fi
          done

          if [[ "$changed" -eq 1 ]]; then
            git commit -m "ci: refresh commit SHAs from image vcs-ref labels"
            # Supply token only at push time — never persisted to .git/config
            git push "https://x-access-token:${GITHUB_TOKEN}@github.com/${{ github.repository }}.git" HEAD
          else
            echo "No effective changes in commit env files, skipping commit."
          fi
        env:
          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

The job-level permissions: contents: write stays as-is — it governs what GITHUB_TOKEN is allowed to do, which is unchanged. This is the same pattern used in #3202 and aligns with the GitHub Security Lab guidance on preventing pwn requests.

Copy link
Copy Markdown
Member

@jiridanek jiridanek Apr 9, 2026

Choose a reason for hiding this comment

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

checkout@v6, release notes:

Improved credential security: persist-credentials now stores credentials in a separate file under $RUNNER_TEMP instead of directly in .git/config

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Apr 9, 2026

Choose a reason for hiding this comment

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

@jiridanek — you're right, and I stand corrected. persist-credentials in v6 now stores credentials in a separate file under $RUNNER_TEMP instead of directly in .git/config, which directly addresses the concern I raised. The token is no longer sitting in a workspace-accessible file where a compromised build tool could trivially read it.

Since the workflow already pins actions/checkout at the SHA corresponding to v6.0.2, the persist-credentials: true + git push pattern here is fine as-is. The x-access-token push workaround I suggested was the right advice for v4/v5, but is redundant with v6's new storage location. Withdrawing that suggestion.

📝 Learnings were identified and not saved (knowledge base disabled). Enable


- name: Install Skopeo CLI
uses: ./.github/actions/apt-install
with:
packages: skopeo

- name: Setup uv and Python
uses: ./.github/actions/setup-uv

- name: Update manifests/base/commit-latest.env
id: update_env
run: |
python3 scripts/update-commit-latest-env.py
./uv run python scripts/update-commit-latest-env.py

- name: Commit and push changes
run: |
Expand Down
4 changes: 2 additions & 2 deletions scripts/update-commit-latest-env.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ async def inspect(images_to_inspect: typing.Iterable[str]) -> list[tuple[str, st


async def main():
with open(PROJECT_ROOT / "manifests/base/params-latest.env", "rt") as file:
with open(PROJECT_ROOT / "manifests/odh/base/params-latest.env", "rt") as file:
images_to_inspect: list[list[str]] = [line.strip().split('=', 1) for line in file.readlines()
if line.strip() and not line.strip().startswith("#")]

Expand All @@ -116,7 +116,7 @@ async def main():
_, commit_hash = result
output.append((re.sub(r'-n$', "-commit-n", variable), commit_hash[:7]))

with open(PROJECT_ROOT / "manifests/base/commit-latest.env", "wt") as file:
with open(PROJECT_ROOT / "manifests/odh/base/commit-latest.env", "wt") as file:
for line in sorted(output):
print(*line, file=file, sep="=", end="\n")

Expand Down
Loading