Repository navigation
fix: enforce token↔host binding; resolve permission scopes against desired ∪ state with apply-time re-resolution - #39
Merged
Conversation
…#30) authedSession paired resolveConfig() (CT_HOST env precedence) with a token read that discarded the stored host, so a CT_HOST override sent the stored login token — as a URL query param — to a foreign host, leaking it into that server's access logs even on a failed login. Compare the stored host against the resolved host BEFORE any network call and refuse on mismatch, naming both hosts. An explicit CT_LOGINTOKEN env token has no stored binding and is exempt. The token still rides as a query param: the handshake requires it (an Authorization header yields a null CSRF token and breaks writes, per ctClient's header comment) — documented at the call site.
…er execute (#29, #33 item 3) Two halves of one fix, sharing a re-resolution point: #29 — a config declaring a group AND a grant scoped to it could never be planned or applied: resolveScope threw for any key absent from state, and it runs (via desiredTuples → buildPermissionPlan) BEFORE executePlan creates anything. Now scope keys resolve against DESIRED ∪ STATE: a key declared in the config but not yet created is valid and renders as pending (`scope=[<key> (created this apply)]`) instead of aborting even read-only `ct plan`. Truly unknown keys (neither declared nor in state) still hard-fail. #33 item 3 — buildPermissionPlan resolved dataIds from PRE-apply state, so a group recreated in the same apply had its grant PUT with the old dangling id. Every scoped tuple now retains its symbolic scopeKey; applyPermissionPlan re-resolves each against the POST-execute state just before writing, so grants always carry fresh ids. A pending tuple that reaches the writer un-resolved is refused rather than silently written as a global grant. buildPermissionPlan gains the desired resources (for the declared-group set); apply passes post-execute state to applyPermissionPlan.
This was referenced Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes a security bug and the permission-scope deadlock from the 2026-07-08 codebase review.
#30 (security) — stored token sent to CT_HOST-overridden host
authedSessionnow reads the stored credentials (host + token) and refuses — before any network call — when the stored token's host differs from the resolved host, naming both hosts and pointing atct auth login. An explicitCT_LOGINTOKENenv token has no stored binding and is exempt. The query-param token handshake stays (codebase evidence: an Authorization header yields a null CSRF token and breaks writes) — documented at the call site; the binding check makes it safe.#29 + #33 item 3 — scope-resolution deadlock and stale dataIds
One shared re-resolution design:
scope=[<key> (created this apply)]; truly unknown keys remain a hard error. Read-onlyct planno longer aborts.docs/permissions.mdscope-resolution section rewritten (the old text documented the deadlock as a deliberate constraint).Verification: 225 passed / 4 skipped, typecheck + lint clean.
Closes #30. Closes #29.
Completes #33 (items 1, 2, 4 in #38).