fix(terminal): skip ValidateToken when org_token_id in context — PR #2025 regression - #2028
fix(terminal): skip ValidateToken when org_token_id in context — PR #2025 regression#2028molecule-ai[bot] wants to merge 1 commit into
Conversation
…ing (KI-005 followup)
There was a problem hiding this comment.
Review — LGTM with a drive-by observation
Changes are correct. The fix properly handles the regression introduced by PR #2025's ValidateToken addition.
Why the fix is right
When WorkspaceAuth middleware validates an org-scoped token (from org_api_tokens), it sets org_token_id in context but does NOT set a workspace_auth_tokens row. The subsequent ValidateToken call for the claimed workspace identity will always return ErrInvalidToken for org tokens — but that doesn't mean unauthorized. The org middleware already validated it. The added guard (if c.GetString("org_token_id") == "") correctly distinguishes "org token already validated" from "no token at all."
Drive-by
The plugin loader rename (pluginloader → githubappauth) is incidental to this PR. Not a blocker.
Merge readiness
mergeable_state: clean — no blocking checks. Approve.
There was a problem hiding this comment.
QA + Security Review
PR #2028 — skip ValidateToken when org_token_id in context (KI-005 follow-up)
Ports PR #2025 fix from staging → main.
Fix: When WorkspaceAuth validated an org-scoped API token (org_token_id set in gin context), HandleConnect now skips the per-workspace ValidateToken call and trusts the org-level assertion. Hierarchy is still enforced by canCommunicateCheck.
Security logic:
- Org tokens validated at org level by WorkspaceAuth (orgtoken.Validate)
- org_token_id set only when orgtoken.Validate succeeds
- If caller presents org token + X-Workspace-ID → ValidateToken would fail (org tokens not in workspace_auth_tokens)
- Fix: check org_token_id first → if set, skip ValidateToken → trust canCommunicateCheck
- Correct: empty org_token_id still triggers ValidateToken + canCommunicateCheck
Test: TestKI005_OrgToken_SkipsValidateToken — stubs canCommunicateCheck to return true for same-org caller, asserts 503 nil-docker (proves ValidateToken was skipped). Clean test.
Note: This is a follow-up to PR #2025 which was merged to staging (78f8391). This PR should also be merged before any E2E run that exercises org-token A2A routing to main.
CI: Platform (Go) ✅, CodeQL ✅
Recommendation: APPROVE.
Cherry-pick of PR #2025 fix — regression in HandleConnect where org-scoped tokens caused 401 on cross-workspace terminal/A2A routing.
Root cause: PR #1885 introduced ValidateToken binding in HandleConnect. Org-scoped tokens (org_api_tokens table) are not in workspace_auth_tokens, so ValidateToken always returned ErrInvalidToken for them → hard 401 for all A2A routing using org tokens.
Fix: If WorkspaceAuth already validated an org token (org_token_id set in gin context), skip the workspace_auth_tokens lookup and trust the X-Workspace-ID claim. Hierarchy enforcement via canCommunicateCheck is unchanged.
Test: TestKI005_OrgToken_SkipsValidateToken regression guard.