kaizen: Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました (#330) - #340
Conversation
…per 経由で利用するよう修正しました (#330)
|
Warning Review limit reached
Next review available in: 35 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository: kaizen-agents-org/coderabbit/.coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (24)
📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThe change moves Git publication behind a credential-aware supervising boundary. It validates the target repository, isolates HTTPS and SSH credentials, resolves trusted Git executables, updates orchestration call sites, adds publication preflight checks, and expands regression coverage. ChangesCredential-safe Git publication
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Orchestrator
participant GitClient
participant TemporaryBareClone
participant GitHub
Orchestrator->>GitClient: Push ref with expectedRepo
GitClient->>GitClient: Validate origin push URL
GitClient->>TemporaryBareClone: Create temporary bare clone
TemporaryBareClone->>GitHub: Publish with isolated credentials
GitHub-->>TemporaryBareClone: Return push result
TemporaryBareClone-->>GitClient: Update local tracking state
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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: 1a82d9f3a0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 167b023dd2
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 888f3c1d5f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f70d78fdc
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bbdfcab5ef
ℹ️ 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".
PR Guardian final report — pass 1/5
The PR was not merged. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d057e246f
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c22737774b
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 390669d621
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fdfe7c1231
ℹ️ 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".
PR Guardian final report — pass 2/5
The PR was not merged. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68edc80961
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab2dba97b0
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f310ed7bff
ℹ️ 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".
|
@codex review |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
test/workspace.test.ts (1)
40-60: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover a populated
--force-with-leasevalue.The runner returns an empty
stdoutforrev-parse --verify refs/remotes/origin/<ref>, so the lease value atsrc/workspace/git.ts:254is the empty string and line 57 asserts--force-with-lease=refs/heads/kaizen/issue-12-retry-branch:.An empty lease value is the no-remote-branch case. The case that protects an existing remote branch carries the remote-tracking sha. The PR objectives require preserving the
force-with-leaseprotection, and that path has no assertion. A regression that drops the sha would still pass this test.Add a case where
rev-parse --verifyreturns a sha and assert the lease contains it.💚 Proposed test
+ it('leases against the recorded remote-tracking commit when one exists', async () => { + vi.stubEnv('GH_TOKEN', 'supervisor-token'); + const runner = vi.fn<CommandRunner>(async (command, args) => ({ + command, + args, + cwd: '/workspace', + exitCode: 0, + stdout: args.join(' ') === 'remote get-url --push --all origin' + ? 'https://github.com/o/r.git\n' + : args[0] === 'rev-parse' && args[1] === '--verify' + ? 'deadbeefdeadbeefdeadbeefdeadbeefdeadbeef\n' + : '', + stderr: '', + durationMs: 1 + })); + + await new GitClient(runner, '/workspace', '/trusted/git') + .push('kaizen/issue-12-retry-branch', { forceWithLease: true, expectedRepo: 'o/r' }); + + const push = runner.mock.calls.find(([, args]) => args[0] === 'push'); + expect(push?.[1]).toContain( + '--force-with-lease=refs/heads/kaizen/issue-12-retry-branch:deadbeefdeadbeefdeadbeefdeadbeefdeadbeef' + ); + });🤖 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 `@test/workspace.test.ts` around lines 40 - 60, Extend the workspace GitClient tests around the existing forceWithLease push case to return a non-empty SHA for the rev-parse --verify remote-tracking ref, then assert the generated push arguments include that SHA in --force-with-lease. Keep the existing empty-lease scenario and add a separate populated-lease case covering an existing remote branch.
🤖 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 `@docs/02-cli-spec.md`:
- Line 72: Update initProject’s preflight validation to require GH_TOKEN or
GITHUB_TOKEN before any configuration, label, workspace, or registry mutation,
using the existing ConfigError path so failures exit with code 2; otherwise
remove the token requirement from the CLI specification.
In `@src/workspace/git.ts`:
- Around line 280-282: Update the finally cleanup around the publication flow in
git.ts so an fs.rm failure cannot replace the original publication, push, or
validation error. Catch cleanup failures from fs.rm and report them separately
while preserving propagation of the original error; retain recursive forced
cleanup behavior.
- Around line 246-251: Add coverage in the publication test around the existing
Git command mock in test/workspace.test.ts, making the git grep inspection
return exit code 2 while preserving normal behavior for other commands. Assert
that publication throws the “Could not inspect” error for the target ref and
verify the push command is never invoked, preserving the fail-closed branch in
the Git LFS inspection flow.
In `@test/command.test.ts`:
- Around line 113-116: Add coverage in the gitPublicationEnv tests for the
startup-captured fallback: call gitPublicationEnv with an empty source and a
non-empty initialToken, then assert KAIZEN_GIT_PASSWORD equals that token. Keep
the existing GITHUB_TOKEN source and missing-token cases unchanged.
In `@test/doctor.test.ts`:
- Around line 110-130: Extend the doctor tests with a GITHUB_TOKEN-only case:
after setupProject, clear GH_TOKEN, set a non-empty GITHUB_TOKEN, run
doctorProject with the existing runner configuration, and assert the publication
auth check succeeds. Ensure the assertion verifies the check named “publication
auth” has ok: true, preserving coverage for the either-variable authentication
contract.
In `@test/workspace.test.ts`:
- Around line 129-139: Update the workspace test’s GIT_SSH_COMMAND assertion to
expect the platform-specific value selected by process.platform, matching the
neighboring assertions. In the runner mock setup, return a realistic non-empty
SHA for the rev-parse command, then update the updateRef expectation to use that
SHA instead of an empty string.
- Line 89: Update the assertion on publicationPush arguments to join the
argument array into a single string before checking that it does not contain
“supervisor-token”, ensuring embedded occurrences within URLs or other arguments
are detected.
---
Outside diff comments:
In `@test/workspace.test.ts`:
- Around line 40-60: Extend the workspace GitClient tests around the existing
forceWithLease push case to return a non-empty SHA for the rev-parse --verify
remote-tracking ref, then assert the generated push arguments include that SHA
in --force-with-lease. Keep the existing empty-lease scenario and add a separate
populated-lease case covering an existing remote branch.
🪄 Autofix
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: Repository: kaizen-agents-org/coderabbit/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 424a5759-6cb0-4e0f-afe1-f3c582f74a11
⛔ Files ignored due to path filters (7)
dist/commands/doctor.jsis excluded by!**/dist/**dist/commands/doctor.js.mapis excluded by!**/dist/**,!**/*.mapdist/utils/command.d.tsis excluded by!**/dist/**dist/utils/command.jsis excluded by!**/dist/**dist/utils/command.js.mapis excluded by!**/dist/**,!**/*.mapdist/workspace/git.jsis excluded by!**/dist/**dist/workspace/git.js.mapis excluded by!**/dist/**,!**/*.map
📒 Files selected for processing (9)
README.mddocs/01-architecture.mddocs/02-cli-spec.mdsrc/commands/doctor.tssrc/utils/command.tssrc/workspace/git.tstest/command.test.tstest/doctor.test.tstest/workspace.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3803bf169c
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a12ee4d3a8
ℹ️ 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".
PR Guardian final report — pass 3/5
The PR was not merged. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1a04c291a
ℹ️ 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 (!process.env.GH_TOKEN && !process.env.GITHUB_TOKEN) { | ||
| throw new ConfigError('Set GH_TOKEN or GITHUB_TOKEN in the supervisor environment before initialization.'); |
There was a problem hiding this comment.
When a builder writes a replacement gh into a writable directory earlier in PATH (the documented setup prepends $KAIZEN_HOME/bin), the post-push PR flow reaches GitHubClient.gh at src/github/client.ts:503, resolves the bare command again, and passes this newly mandatory token through githubCliEnv(). The replacement can therefore capture the supervisor credential and fake privileged GitHub operations. Fresh evidence beyond the resolved publication-helper finding is this ordinary post-build GitHubClient path, which never uses a pinned executable; resolve a trusted immutable gh before untrusted work and use it for every privileged GitHub call.
AGENTS.md reference: AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
1 participant
Closes #330
元Issue
#330: Preserve Git publication credentials across isolated builder execution
Summary
A manual recovery run for kaizen-loop#319 completed implementation and verification, then failed only at git push because the isolated builder subprocess could not read credentials for the HTTPS origin. The supervising Kaizen process had GitHub access and successfully published the unchanged generated commit afterward.
Evidence
…
Builder task understanding
Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。
builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
Builder notes
回帰テストを追加し、builder に GH_TOKEN が渡らず supervisor が PR publication まで完了することを確認しました。認証付き push は pre-push hook を実行しません。指定検証は全て成功（587 passed、1 skipped）。dist を更新済みで、保護対象パスの変更はありません。
Provider evidence:
Selected backend: codex
Final payload source: last-message
変更ファイル
dist/utils/command.d.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
dist/utils/command.js— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
dist/utils/command.js.map— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
dist/workspace/git.js— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
dist/workspace/git.js.map— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
src/utils/command.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
src/workspace/git.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
test/command.test.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
test/improve.test.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
test/integration/dry-run.test.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
test/workspace.test.ts— Git publication 時のみ supervisor の GitHub 認証を安全な credential helper 経由で利用するよう修正しました。builder には認証情報を渡さず、force-with-lease と ready-for-review PR の既存保護を維持しています。
Changed files: 11 / Changed lines: 121
Verification
npm test— 成功npm run typecheck— 成功npm run check:dist— 成功npm run build— 成功test -f skills/gh-link-issue-pr/SKILL.md && test -f skills/kaizen-bug-router/SKILL.md && test -f skills/pr-guardian/SKILL.md— 成功Verifier verdict
verifier: open_pr_with_warning
summary: Open PR with warning and 2 should_fix item(s); risk is medium.
evidence: reported (未実行の可能性あり)
should_fix: [verify_logs] Verification output contains a non-blocking risk signal. — evidence: Tests 587 passed | 1 skipped (588)
should_fix: [builder_report] Verification output contains a non-blocking risk signal. — evidence: 回帰テストを追加し、builder に GH_TOKEN が渡らず supervisor が PR publication まで完了することを確認しました。認証付き push は pre-push hook を実行しません。指定検証は全て成功（587 passed、1 skipped）。dist を更新済みで、保護対象パスの変更はありません。
confidence: 60/100
risk: medium
notes: evidence_grade=reported
warning: この判定は実行証拠ではなくテキスト報告に基づくため、未実行の可能性があります。
Evidence strength
残存リスク / レビュー観点
Verifier cleared PR with warning: Open PR with warning and 2 should_fix item(s); risk is medium.
should_fix: [verify_logs] Verification output contains a non-blocking risk signal. — evidence: Tests 587 passed | 1 skipped (588)
should_fix: [builder_report] Verification output contains a non-blocking risk signal. — evidence: 回帰テストを追加し、builder に GH_TOKEN が渡らず supervisor が PR publication まで完了することを確認しました。認証付き push は pre-push hook を実行しません。指定検証は全て成功（587 passed、1 skipped）。dist を更新済みで、保護対象パスの変更はありません。
confidence: 60/100
risk: medium
Summary by CodeRabbit
Bug Fixes
Security
Documentation