Skip to content

fix(ci): avoid persisted credentials for public submodules#2535

Open
YOMXXX wants to merge 3 commits into
tinyhumansai:mainfrom
YOMXXX:fix/ci-public-submodule-checkout
Open

fix(ci): avoid persisted credentials for public submodules#2535
YOMXXX wants to merge 3 commits into
tinyhumansai:mainfrom
YOMXXX:fix/ci-public-submodule-checkout

Conversation

@YOMXXX
Copy link
Copy Markdown
Contributor

@YOMXXX YOMXXX commented May 23, 2026

Summary

Problem

  • Several CI jobs fetch public submodules with actions/checkout@v5 and submodules: recursive.
  • The checkout step can inject the current repository token as a Git extraheader for https://github.com/*.
  • In failing PR runs, submodule clones for tinyhumansai/tauri-cef and tinyhumansai/tauri-plugin-notification failed before build/test code ran with fatal: could not read Username for 'https://github.com': terminal prompts disabled.
  • Appium E2E also failed before running code because the job container could not log into ghcr.io to pull ghcr.io/tinyhumansai/openhuman_ci:latest from a fork PR token with no package scope.

Solution

  • Set persist-credentials: false on recursive submodule checkout steps in PR-facing build/test workflows.
  • Add packages: read to workflows that use the shared GHCR CI image.
  • Keep release/signing workflows unchanged so their dedicated credential chain is not affected.
  • Verified the underlying access pattern locally: anonymous git ls-remote works for both public submodules; a bad GitHub extraheader reproduces failure exit 128.

Submission Checklist

  • Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy. N/A: GitHub Actions checkout/container-permission configuration change; validated via YAML parse and git access repro.
  • Diff coverage >= 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. N/A: workflow-only change; CI gate will still run.
  • Coverage matrix updated — added/removed/renamed feature rows in docs/TEST-COVERAGE-MATRIX.md reflect this change. N/A: CI infrastructure only.
  • All affected feature IDs from the matrix are listed in the PR description under ## Related. N/A: no feature ID changed.
  • No new external network dependencies introduced (mock backend used per Testing Strategy)
  • Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md). N/A: PR CI checkout only; release workflows left unchanged.
  • Linked issue closed via Closes #NNN in the ## Related section. N/A: no dedicated issue exists; discovered while babysitting PR CI.

Impact

  • CI-only impact.
  • PR jobs should fetch public vendored submodules anonymously instead of with a repository-scoped extraheader that can be rejected for sibling repositories.
  • PR jobs using ghcr.io/tinyhumansai/openhuman_ci:* receive explicit package read permission.
  • Release/signing workflows are unchanged.

Related

  • Follow-up PR(s)/TODOs: rerun or retrigger affected PRs after this CI fix lands if their only remaining failures are submodule checkout or GHCR container auth failures.

AI Authored PR Metadata (required for Codex/Linear PRs)

Linear Issue

  • Key: N/A
  • URL: N/A

Commit & Branch

  • Branch: fix/ci-public-submodule-checkout
  • Commit SHA: 88ae50ff

Validation Run

  • pnpm --filter openhuman-app format:check — N/A: no app/source formatting changed.
  • pnpm typecheck — N/A: no TypeScript changed.
  • Focused tests: git ls-remote anonymous vs bad-extraheader repro for tinyhumansai/tauri-cef and tinyhumansai/tauri-plugin-notification; docker manifest inspect ghcr.io/tinyhumansai/openhuman_ci:latest
  • Rust fmt/check (if changed): N/A: no Rust changed.
  • Tauri fmt/check (if changed): N/A: no Tauri source changed.

Validation Blocked

  • command: N/A
  • error: N/A
  • impact: N/A

Behavior Changes

  • Intended behavior change: PR CI recursive submodule checkouts avoid persisted repository credentials; PR workflows can read shared GHCR CI images.
  • User-visible effect: none at runtime; CI reliability improvement.

Parity Contract

  • Legacy behavior preserved: recursive submodule checkout remains enabled in the same jobs; GHCR image names are unchanged.
  • Guard/fallback/dispatch parity checks: release/signing workflows are not modified.

Duplicate / Superseded PR Handling

  • Duplicate PR(s): N/A
  • Canonical PR: N/A
  • Resolution (closed/superseded/updated): N/A

Summary by CodeRabbit

  • Chores
    • Tightened CI security: workflows now grant explicit package read access and disable credential persistence during repository checkouts across build, coverage, test, and E2E pipelines.

Review Change Stack

@YOMXXX YOMXXX requested a review from a team May 23, 2026 13:00
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5e0d1846-8a4d-4ff9-8577-1812df320261

📥 Commits

Reviewing files that changed from the base of the PR and between b4c67f1 and 88ae50f.

📒 Files selected for processing (8)
  • .github/workflows/build.yml
  • .github/workflows/coverage.yml
  • .github/workflows/e2e-reusable.yml
  • .github/workflows/e2e.yml
  • .github/workflows/pr-quality.yml
  • .github/workflows/test-reusable.yml
  • .github/workflows/test.yml
  • .github/workflows/typecheck.yml
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/test.yml

📝 Walkthrough

Walkthrough

Workflow-level packages: read permission was added across multiple GitHub Actions workflows, and several actions/checkout@v5 steps were updated to persist-credentials: false to prevent GITHUB_TOKEN from being persisted in job containers.

Changes

CI workflow updates

Layer / File(s) Summary
Add packages: read to workflow permissions
.github/workflows/build.yml, .github/workflows/coverage.yml, .github/workflows/e2e-reusable.yml, .github/workflows/e2e.yml, .github/workflows/pr-quality.yml, .github/workflows/test-reusable.yml, .github/workflows/test.yml, .github/workflows/typecheck.yml
Added permissions.packages: read at workflow level in multiple workflows to grant package read scope to GITHUB_TOKEN.
Disable credential persistence in checkout
.github/workflows/build.yml, .github/workflows/coverage.yml, .github/workflows/e2e-reusable.yml, .github/workflows/test-reusable.yml
Set persist-credentials: false on actions/checkout@v5 in build, coverage (rust-core, rust-tauri), e2e (linux, rust-linux, macOS, windows), and rust test jobs to avoid persisting GitHub credentials in job containers.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • tinyhumansai/openhuman#1887: Modifies reusable CI workflows and actions/checkout@v5 configuration (persist-credentials: false) similar to this PR.

Poem

🥕 A rabbit tidies workflows with care,
Permissions set and tokens spared,
Checkouts close the credential door,
CI hops safe forevermore. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly addresses the main change: setting persist-credentials to false for public submodules in CI workflows. It accurately reflects the core fix across all modified workflow files.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 23, 2026
@YOMXXX
Copy link
Copy Markdown
Contributor Author

YOMXXX commented May 23, 2026

@graycyrus @senamakel Ready for review.

This PR is green on 88ae50ff (22 success, 2 skipped) and directly addresses the CI infra failures currently blocking other fork PRs:

The new run confirms the fix path: Build Tauri, Test, E2E, and Coverage all passed with the workflow changes, and there are no unresolved review threads.

@YOMXXX
Copy link
Copy Markdown
Contributor Author

YOMXXX commented May 24, 2026

Status update: this CI infrastructure fix is now the blocker for several otherwise-ready PRs. The current failures on #2533/#2544 come from checkout/fetch or vendored submodule auth before tests run, and #2534's E2E failure stops at GHCR job-container login with no secrets. This PR is green and CodeRabbit-approved, so merging it first should unblock reruns for those PRs without duplicating workflow changes into each feature branch.

@YOMXXX
Copy link
Copy Markdown
Contributor Author

YOMXXX commented May 24, 2026

Updated this branch with latest upstream/main to satisfy the up-to-date branch-protection path. The PR diff is still limited to the CI workflow credential/package-read changes, and checks are rerunning now.

@YOMXXX
Copy link
Copy Markdown
Contributor Author

YOMXXX commented May 24, 2026

@graycyrus @senamakel Required checks are all green now, and there are no unresolved review threads. The remaining non-required Windows secrets ACL job is marked cancelled after successful checkout/submodule setup and passing test output in the log. Please review/merge this CI fix first so #2534/#2544 can be updated and rerun on the fixed workflows.

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.

1 participant