fix(security): bind invite acceptance to invited identity - #593
fix(security): bind invite acceptance to invited identity#593seonghobae wants to merge 22 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough요청 로그에서 초대 및 공유 토큰을 Changes초대 신원 및 토큰 보안
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: 🟡 Moderate · up to The change prevents token-only invite redemption and improves secret-path logging, but an invite can still be redeemed by a replacement account after the original invited account is deleted, and equivalent Cherokee email forms may be treated inconsistently. The current head also lacks all passing required checks and qualifying approval, so it is not merge-ready. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Client as API 요청자
participant App as server/app.mjs
participant Database as SQLite 사용자 및 초대 데이터
participant Logs as 요청 로그
Client->>App: 토큰 포함 요청 전송
App->>Logs: 토큰을 :token으로 치환한 경로 기록
Client->>App: 초대 수락 요청 전송
App->>Database: 정규화 이메일과 인증 사용자 조회
Database-->>App: 사용자 조회 결과 반환
App-->>Client: 조건 충족 시 수락, 아니면 404
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 6 files. (3 skipped: 3 unsupported.) ✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
|
Superseded by the established #589 security lane after exact-head comparison. #5892d39860 already carries the same executable #592 contract: viewer-visible roster responses strip pending invite bearer tokens; mismatched authenticated identities receive the generic invalid/used response before membership mutation; mixed-case intended identities can redeem once; replay is rejected. Its |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent review Review-only request for exact head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent review Review-only request for exact current contributor head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent review Please review exact current head Verify from current source before issuing a SHA-bound verdict:
If the Unicode canonicalization policy is too broad or incomplete, identify the smallest standards-defensible identity rule and a realistic RED case rather than accepting an ad-hoc partial case-fold table. |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
README.md— repository behaviorpackage-lock.json— repository behaviorpackage.json— repository behaviorserver/app.mjs— TypeScript or JavaScript runtimeserver/db.mjs— TypeScript or JavaScript runtimeserver/email_identity.mjs— TypeScript or JavaScript runtimetests/api/invite-identity-security.test.mjs— regression suitetests/unit/coverage-script-contract.test.mjs— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: README.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: README.md"]
R1 --> V1["required checks"]
Evidence --> S2["Repository file: package-lock.json"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Repository file: package-lock.json"]
R2 --> V2["required checks"]
Evidence --> S3["Repository file: package.json"]
S3 --> I3["repository behavior"]
I3 --> R3["Review risk: Repository file: package.json"]
R3 --> V3["required checks"]
Evidence --> S4["TypeScript/JavaScript: app.mjs (3 files)"]
S4 --> I4["TypeScript or JavaScript runtime"]
I4 --> R4["Review risk: TypeScript/JavaScript: app.mjs (3 files)"]
R4 --> V4["package test plus coverage"]
Evidence --> S5["Test: invite-identity-security.test.mjs"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test: invite-identity-security.test.mjs"]
R5 --> V5["targeted test run"]
Evidence --> S6["Test: coverage-script-contract.test.mjs"]
S6 --> I6["regression suite"]
I6 --> R6["Review risk: Test: coverage-script-contract.test.mjs"]
R6 --> V6["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
27e581a7298b9faa309d9c838e1f5f05d3c0f355 - Workflow run: 33232630124
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: README.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: README.md"]
R1 --> V1["required checks"]
Evidence --> S2["Repository file: package-lock.json"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Repository file: package-lock.json"]
R2 --> V2["required checks"]
Evidence --> S3["Repository file: package.json"]
S3 --> I3["repository behavior"]
I3 --> R3["Review risk: Repository file: package.json"]
R3 --> V3["required checks"]
Evidence --> S4["TypeScript/JavaScript: app.mjs (3 files)"]
S4 --> I4["TypeScript or JavaScript runtime"]
I4 --> R4["Review risk: TypeScript/JavaScript: app.mjs (3 files)"]
R4 --> V4["package test plus coverage"]
Evidence --> S5["Test: invite-identity-security.test.mjs"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test: invite-identity-security.test.mjs"]
R5 --> V5["targeted test run"]
Evidence --> S6["Test: coverage-script-contract.test.mjs"]
S6 --> I6["regression suite"]
I6 --> R6["Review risk: Test: coverage-script-contract.test.mjs"]
R6 --> V6["targeted test run"]
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
|
@opencode-agent review Review exact current head |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@opencode-agent review Please review exact current contributor head Fresh current state: every inline review thread is resolved; repository-owned Dependency Review, Fuzz, Security Scan, OSV Scanner, Server Tests, and SAST Semgrep are terminal-success on this exact head. The required OpenCode workflow failed only because no authenticated Verify the current source independently, especially canonical invite identity uniqueness, full case-fold equivalence including Cherokee regressions, existing-account invite creation, secret-bearing invite/share path redaction, revoked-token behavior, and no pre-identity membership/audit/delivery/invite-consumption mutation. Anchor any formal verdict to this exact head. |
|
@opencode-agent review Review only exact current contributor head |
|
Hour-20 exact-head review request. Current head Invite identity binding. Land only with two independent APPROVE reviews plus exact-head Checks. Do not self-approve. Do not --admin merge. Checks/reviews are not a reason to weaken fail-closed gates. |
|
Hour-21 exact-head review request. Current head @opencode-agent review |
|
Hour-22 exact-head review request. Invite identity bind. Current head @opencode-agent review |
|
Hour-23 exact-head review request. Current head @opencode-agent review |
1 participant
Fixes #592.
Focused security hotfix from protected
develop@2c328875e00e86537df3e965170be80532571cad.Exact contributor head:
a7bc39d16fe5240832ef08f0ee80e9c64dea7181.Verified protected defects
Protected
developexposes pending invite bearer tokens inGET /api/orgs/:id/membersand allows any authenticated token-holder to redeemPOST /api/invites/:token/accept, enabling cross-identity privilege acquisition.The existing
users.email TEXT UNIQUErule is case-sensitive, so historical accounts that differ only by email case can coexist. A simple lowercased equality check is therefore insufficient: either colliding account could otherwise redeem the same canonical invite.The structured request logger also persisted raw secret-bearing paths. Current-head review further proved that endpoint-shaped regex redaction was insufficient because malformed/trailing paths such as
/api/invites/TOKEN/accept/extra,/api/invites/TOKEN/accept/, and/api/shared/TOKEN/extracould still copy live bearer material into durable logs.TDD repair
tests/api/invite-identity-security.test.mjscovers:token;404 {"error":"invalid or used invite"}and gains no membership;develop's existing fail-closed404behavior;RED sequencing is retained in predecessor test-only commits rather than promoted as current passing evidence: canonical-account collision in
bebc2074a276eafc62de6e36134ae91d7abe2d0c, and request-log redaction strengthening in2333d32874d3ab10a83071e075d4e39f2daf6b60/d0e1a732d63d2da0c9a2b987ea00a556b029bafc. Their hosted runs were cancelled by branch movement/concurrency, so only source/test sequencing is claimed from them.The production repair on current head:
tokenfrom the pending-invite roster projection;uid;api/invitesandapi/shared, preserving any suffix for operational route-shape visibility while removing the bearer value.Fresh current-source verification confirms the latest malformed-path redaction finding is addressed on
a7bc39d16fe5240832ef08f0ee80e9c64dea7181; all inline review threads are resolved. The latest informational review observation correctly notes that zero or multiple canonical-email matches fail closed and does not require a source change.Current exact-head evidence boundary
For exact contributor head
a7bc39d16fe5240832ef08f0ee80e9c64dea7181, GitHub currently reports terminal success for Dependency Review33204300753, Fuzz33204300875, OSV Scanner33204301500, SAST Semgrep33204301020, Security Scan33204300744, Server Tests33204300763, Required Noema Review33204299303, Close Empty PR33204299417, Required PR Review Merge Scheduler33204299299, and the dynamic CodeQL/code-quality checks.Those successful workflow conclusions are not all promoted to merge-authorizing exact-contributor-checkout evidence. Repository Server Tests still requires the exact-head checkout/attestation correction owned by ScopeWeave PR #523 before a green result can prove the tested checkout contract rather than only the PR-associated run. Central reusable exact-head evidence defects remain owned by the existing
.githubrepair/control lanes.Two inherited required gates are explicitly non-passing on this exact head: Required Strix Security Scan run
33204299340completedcancelled, and Required OpenCode Review run33204299375completedfailure. Required Noema is exact-head success, but no formal review submission is a qualifying independent current-headAPPROVEDreview.Lane history
This focused lane was previously closed only because #589 carried the same behavior. #589 is closed unmerged while protected
developstill has the defect, so #593 remains the narrow current owning lane. #587 does not contain this lane's invite/share request-log bearer redaction, so #593 is not semantically superseded.Merge boundary
Do not merge or enable auto-merge until the unchanged exact contributor head remains reconciled to live protected
develop, all applicable repository and inherited required checks are substantively terminal-passing under corrected evidence contracts, current valid unresolved review findings are zero, and live branch protection/rulesets have the required qualifying independent current-head approval. Pending, queued, skipped-required, cancelled, absent, neutral-required, failed, stale, predecessor, synthetic-only, status-only, author-only, model-only, rate-limited, or infrastructure-only evidence is non-passing.Summary by CodeRabbit
보안 개선
문서