chore(web): remove unused Bearer-token auth helpers#225
Closed
karthikarunapuram8-dot wants to merge 1 commit into
Closed
chore(web): remove unused Bearer-token auth helpers#225karthikarunapuram8-dot wants to merge 1 commit into
karthikarunapuram8-dot wants to merge 1 commit into
Conversation
Follow-up to alookai#210. dual-auth.ts (requireAuth) and api-auth.ts (withToken) accepted any non-null row from getMachineTokenByToken without checking mt.status, the same pattern the auth middleware fix in alookai#210 closed. Both helpers currently have zero callers in the repo, so this is a latent gap rather than a live exploit, but keeping the duplicated and unsafe validation logic around invites silent regression if either helper is ever wired up later. Deleting both. withAuth in src/web/src/lib/middleware/auth.ts remains the single source of truth for Bearer-token validation. Verified zero references via grep on (dual-auth, api-auth, requireAuth, withToken) across the whole repo. Typecheck passes.
Author
|
Re-verified the gauntlet on this branch after the deletion:
Branch is also still up to date with |
9 tasks
Contributor
|
Thanks for catching this! These files have already been removed as part of #235, which added automated dead code detection (knip) across the entire codebase and cleaned up all existing unused code in one pass. Closing as superseded — appreciate the contribution! |
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
2 participants
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.
Follow-up to #210, addressing the reviewer's flagged latent gap.
Context
In their review of #210, @anky98ai pointed out that the same accept-any-non-null pattern existed in two other Bearer-token entry points without a status guard:
src/web/src/lib/dual-auth.ts(requireAuth)src/web/src/lib/api-auth.ts(withToken)Both would admit a pending machine token exactly as
withAuthdid before #210.Why deletion rather than hardening
Both helpers have zero callers anywhere in the repo. Verified via grep on the helper file names, the function names, and the call-site patterns (
requireAuth(,withToken().Keeping unused, duplicated, and unsafe validation logic around invites silent regression if either helper is ever wired up later. Deleting both eliminates the latent gap rather than hardening dead code that may stay dead.
withAuthinsrc/web/src/lib/middleware/auth.tsremains the single source of truth for Bearer-token validation, and now there is no other path that could quietly reintroduce the pending-token-accepted bug.Scope
-52lines, no additionspnpm typecheckpasses after the deletionIf a future use case calls for Bearer-token auth outside the existing
withAuthwrapper, the right move is to reusewithAuth(or extract its validation into a shared helper) rather than reintroducing parallel implementations.Happy to take a different approach if you would prefer hardening over deletion.