Skip to content

Stop the VOD loader from logging user data and hiding Twitch errors - #83

Merged
t3dotgg merged 2 commits into
mainfrom
t3code/private-marker-access
Sep 25, 2026
Merged

t3dotgg merged 2 commits into
mainfrom
t3code/private-marker-access

Conversation

@t3dotgg

@t3dotgg t3dotgg commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

The VOD page loader had three problems:

  • It logged the Clerk user object and the full Twitch responses. This put personal data in the server logs.
  • When the Twitch markers request failed, the page showed an empty marker list. The export was wrong, and nothing told you.
  • When Twitch sent an empty videos list, the page crashed.

Now the loader does not log that data. A failed Twitch request throws with its status code, so you see the error page and not an empty list. An empty videos list gives no markers.

Marker access does not change. The page still reads markers with the creator's stored Twitch token, so any signed-in user can see a connected creator's markers. This is intentional (see bf5e5c5 and the demo link from #58). The first version of this PR used the viewer's token and showed a "markers are private" page. This version removes that change and keeps the 60 second revalidate.

#81 rewrites the same loader and includes these fixes. When it rebases, it can keep its own loader.

Tests: pnpm test (33 passed), pnpm typecheck, pnpm lint, and pnpm build. The two new tests fail on main.

Created with GPT-6 Astra in Codex. Takeover fixes by Claude Opus 5.5 in Claude Code.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes
    • VOD marker retrieval now uses the creator’s stored Twitch authorization, helping ensure requests use the correct credentials.
    • Marker data may be reused for up to 60 seconds instead of always being fetched fresh.
    • Missing VOD data and unsuccessful VOD or marker responses now result in errors; marker errors include the response status.

@vercel

vercel Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
markerthing Ready Ready Preview Sep 25, 2026 4:08am UTC

Request Review

@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: bb36bac7-f5f6-4fb1-a79d-378a46cd020b

📥 Commits

Reviewing files that changed from the base of the PR and between 3492f40 and 6b47ebd.

📒 Files selected for processing (2)
  • src/utils/twitch-server.test.ts
  • src/utils/twitch-server.ts

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.


Walkthrough

getVodWithMarkers now uses the VOD creator’s stored token for marker requests. It checks VOD and marker responses, applies 60-second revalidation to marker requests, and returns an empty marker list when Twitch returns no markers.

Changes

VOD Marker Flow

Layer / File(s) Summary
Creator-token marker request
src/utils/twitch-server.ts, src/utils/twitch-server.test.ts
The helper checks the VOD response and requires user_login, then uses the creator’s stored token for the marker request. Marker requests use 60-second revalidation, and non-OK responses throw an error containing the status. The exported TwitchMarkerAccessDeniedError class was removed. Tests cover a 401 marker response and a successful response with no markers.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 6b47e

VOD marker loading keeps using the creator's connected Twitch account, as the app documents, and now reports Twitch request failures instead of silently returning empty results. No concrete defect remains. Note that the PR description still describes a viewer-only access rule that the final code intentionally does not implement.

Security Architecture Review

Security architecture risk: 🟠 High · up to 6b47e

Signed-in viewers with a Twitch connection can still receive another connected creator’s private VOD markers. This PR does not introduce that exposure, but it also does not deliver its stated access restriction. It improves error handling and removes sensitive logging.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — A viewer with a usable Twitch connection and a VOD ID can cause the server to request markers under a connected creator’s stored token. The affected data is those creators’ VOD markers, not the token itself; missing creator records and Twitch rejection limit individual requests.

Security Findings and Attack Paths

  • observed — The retained authorization finding describes a signed-in non-owner reaching another creator’s markers through the creator’s stored token. Base and head source show the same authorization path, so it is a serious residual condition rather than an observed new or worsened exposure from this PR.

Trust Boundaries and Controls

  • observed — The page’s sign-in gate and the viewer-token VOD request remain controls, but neither binds marker authorization to the viewer. A live non-OK marker response now throws; that control applies when the request reaches Twitch.

Resilience and Maintainability Implications

  • inferred — The marker fetch retains its pre-existing 60-second revalidation setting. The source and direct 401 test do not establish whether a cached private response can survive token revocation or how the deployed runtime keys and invalidates it; stale access remains a proof gap, not a verified PR regression.

Hardening Proposals

  • proposed — Make marker access depend on the requesting viewer’s owner or editor authority rather than exercising the creator’s stored token on that viewer’s behalf; verify this with a non-owner route case as well as authorized owner and editor cases.
  • proposed — Establish the deployed marker-cache behavior under credential revocation before relying on a 60-second revalidation policy for private markers.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 3 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title describes the error-handling and logging changes in the pull request. It does not mention the token-source change, but it remains specific and related to the changeset.
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Removes debug logging and adds error handling to the VOD loader.

The PR appears safe to merge; no outstanding findings or new actionable issues remain.

Reviews (2) · Last reviewed commit: "Keep creator-token marker access and fix..."

Comment thread src/app/(core)/v/[slug]/page.tsx Outdated
Comment thread src/utils/twitch-server.test.ts Outdated
The VOD page reads markers with the creator's stored token again, as on
main, and the page keeps its 60 second revalidate. Removes the viewer
token path and the "markers are private" page.

Keeps the clear fixes: no more logs of the Clerk user or Twitch payloads,
non-OK Twitch responses throw, and an empty videos list gives no markers.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@t3dotgg t3dotgg changed the title Keep Twitch markers private to owners and editors Stop the VOD loader from logging user data and hiding Twitch errors Sep 25, 2026
@t3dotgg

t3dotgg commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

Note

🤖 Claude Opus 5.5 responding on behalf of Theo

@greptileai review

6b47ebd changes the scope of this PR. Marker access stays as on main, and the PR now only fixes loader logging, Twitch error handling, and the empty videos crash.

@t3dotgg
t3dotgg merged commit 4b485d7 into main Sep 25, 2026
6 checks passed

This branch was successfully deployed

1 active deployment
Preview — 6b47ebdb Deployed Sep 25, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant