Repository navigation
Conversation
…emo for #98) Throwaway commit showing that the protected workflow runs the base copy of the invariant suite. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
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 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request weakens the four-eyes policy threshold from 2 to 99 in packages/kernel/src/approval.ts, which effectively bypasses a critical security control designed to prevent unauthorized self-approvals. Additionally, two key invariant tests in apps/kernel/test/invariant/kernel-invariants.db.test.ts have been skipped, disabling verification for both the four-eyes policy and the revision editing policy. It is highly recommended to revert the threshold change and re-enable these tests to maintain the integrity of the security controls and test coverage.
|
|
||
| const isAuthor = authors.some((a) => a.type === 'user' && a.id === actor.userId); | ||
| if (isAuthor && policy.fourEyes && eligible.size >= 2) return deny('four_eyes', eligible.size); | ||
| if (isAuthor && policy.fourEyes && eligible.size >= 99) return deny('four_eyes', eligible.size); |
There was a problem hiding this comment.
Weakening the four-eyes policy threshold from 2 to 99 allows self-approval of proposals even when multiple eligible approvers exist. This bypasses a critical security control (P10) designed to prevent unauthorized self-approvals.
| if (isAuthor && policy.fourEyes && eligible.size >= 99) return deny('four_eyes', eligible.size); | |
| if (isAuthor && policy.fourEyes && eligible.size >= 2) return deny('four_eyes', eligible.size); |
| } | ||
|
|
||
| it('four-eyes (P10): with two eligible approvers the uploader cannot approve, the other approver can', async () => { | ||
| it.skip('four-eyes (P10): with two eligible approvers the uploader cannot approve, the other approver can', async () => { |
There was a problem hiding this comment.
Skipping this invariant test disables verification of the four-eyes (P10) policy, reducing test coverage for core security guarantees.
it('four-eyes (P10): with two eligible approvers the uploader cannot approve, the other approver can', async () => {| }); | ||
|
|
||
| it('turns an edit into a new revision authored by the editor; the old revision and unsaved changes cannot be approved', async () => { | ||
| it.skip('turns an edit into a new revision authored by the editor; the old revision and unsaved changes cannot be approved', async () => { |
There was a problem hiding this comment.
Skipping this invariant test disables verification of the revision editing policy, which ensures that old revisions and unsaved changes cannot be approved.
it('turns an edit into a new revision authored by the editor; the old revision and unsaved changes cannot be approved', async () => {
|
Demo done: |
This branch was previously deployed
1 participant
Throwaway draft for #98. It weakens four-eyes (P10) in
packages/kernel/src/approval.tsand skips the two invariant tests that would catch it. Expected:ci(PR copy of the tests) passes,Invariant suite (from base)fails because it runs the base branch's copy. Closed and deleted after the run.🤖 Generated with Claude Code