diff --git a/HANDOFF.md b/HANDOFF.md index 4476285..ea1e696 100644 --- a/HANDOFF.md +++ b/HANDOFF.md @@ -59,18 +59,27 @@ re-consent. Parking verification does not fix that; it accepts it for the solo-u real work the moment a *second real user* needs to sign in, or at Store submission (TASK 48). The sensitive-vs-restricted classification was never read and is moot until then. -**These two survive the tabling** — neither needs verification to land, and both shrink the surface -if it ever comes back: -- `Services/YouTubeAuthService.cs:18` requests `auth/youtube` **+ `auth/youtube.force-ssl`**. - `force-ssl` is **redundant** — Google's liveChat docs list `auth/youtube` as an acceptable - alternative, and it is the broader of the two ("Manage your YouTube account" ⊃ "see/edit/delete - your videos, ratings, comments, captions"). Drop it: one fewer scope to justify, one fewer consent line. -- `Services/ChannelStatsService.cs:104,107` — `playlistItems.list` on the creator's own uploads - playlist (`UU…`) is **public data needing no OAuth scope at all** (API key suffices); it currently - rides on the user's session token. Does not change the requested scope (`auth/youtube` is forced by - `liveBroadcasts.*`/`liveStreams.list` regardless) but that fetch should not need user credentials. - The `channels?mine=true` call on the same service **does** need the session — do not "fix" it. -- `auth/youtube` is **unavoidable**: no narrower scope covers live broadcast create/transition. +**One of these two survives the tabling, and the other was a bad idea (corrected 2026-09-28):** +- ✅ **DONE — `Services/YouTubeAuthService.cs` no longer requests `auth/youtube.force-ssl`.** + It is **redundant**: its grant is *"see, edit, and permanently delete your YouTube videos, + ratings, comments and captions"*, a strict subset of `auth/youtube`'s *"Manage your YouTube + account"*, which is the scope we keep. Google's own `liveBroadcasts.insert` reference lists + `auth/youtube` as an accepted **alternative** to `force-ssl`, so every endpoint this app calls + (`channels`, `liveBroadcasts`, `liveStreams`, `liveChat`, `playlistItems`) is covered. + Two consent lines became one. **No forced re-consent** — a refresh token keeps the scopes it + was originally granted, so existing sessions are untouched; the reduced set applies at the + next fresh sign-in. Removing a scope cannot narrow capability when the one kept is broader. +- ❌ **NOT doing this, and the earlier note recommending it was wrong: moving + `playlistItems.list` off the user's session token.** It was written during the verification + hunt on the theory that "public data should not ride on user credentials". Walking the code + kills it: `ChannelStatsService` is built as `new ChannelStatsService(_youtubeAuth)` with **one** + `HttpClient`, and `channels?mine=true` (same service) **genuinely needs the session**. To split + it you would need a second unauthenticated client **plus a Google API key**, because every + YouTube Data API request needs a credential — without a key the call 403s. So the "fix" means + **shipping a new extractable credential in the binary, subject to quota abuse, to replace a + token the app already holds.** It buys **zero** verification-surface reduction (the data is + public; no scope is involved) and zero correctness. Strictly worse. ⛔ Do not "fix" this later + without a new ruling. ## ✅ DECIDED 2026-09-27 — pricing: `$10/mo` subscription, or `$400` lifetime, **no annual** diff --git a/MyMistakes.md b/MyMistakes.md index ae17420..d5fe3e6 100644 --- a/MyMistakes.md +++ b/MyMistakes.md @@ -1314,3 +1314,29 @@ the claim was right but imprecise, and it now says precisely that, with the chec The same file was still naming build tag `autobuild-2026-08-09-13-03` — **the tag that 404'd.** A licence record that names a stale artifact is worse than no licence record, because it reads as verified. When you pin something, the pin and the paperwork must move in the same commit. + +### I repeated, in the same session, the mistake I had just written down + +The ffmpeg-bundling unit added `TheLocatorCarriesNoNetworkCapability`, which failed on its first +run because a file-wide `DoesNotContain("Download")` matched that file's own doc comment — the one +saying "nothing is downloaded". I diagnosed it properly (bare word matches hit prose; use +call-shaped patterns) and wrote the lesson into this file. **Two hours later, in the very next +change, I wrote `DoesNotContain("force-ssl")` against `YouTubeAuthService.cs` and it failed for +the identical reason**: that file now names `force-ssl` in its doc comment *precisely to explain +why it was removed*. Writing the lesson down did not prevent the repeat; the lesson was not yet +load-bearing in the place the decision gets made. + +**The actual fix, and it generalises past source-scan tests:** don't scan the *file* for a +token, **extract the value and assert on the value.** A `Regex` that pulls +`private const string Scope = "([^"]*)";` and then splits the captured string into scopes tests +the thing that matters — "exactly one scope, and it is the broad one" — and is structurally +incapable of matching prose. The general form: **when a fact must be asserted about a literal +that the surrounding documentation is also allowed to discuss, assert on the parsed literal, not +on the text that contains it.** A test that reads a file and greps it is a test whose failure +mode is "the comment changed", and that is how source-scan tests get deleted rather than obeyed. + +**Meta-lesson worth more than the fix:** I made the same class of error twice inside one session +while actively documenting it. Repeating a documented mistake is not a knowledge failure — the +knowledge was fine — it is a *check* failure. The check that would have caught it: when a +`DoesNotContain` is aimed at a file, ask "does this file's own documentation have a legitimate +reason to contain the string?" If yes, the assertion is aimed at the wrong target. diff --git a/Services/YouTubeAuthService.cs b/Services/YouTubeAuthService.cs index f12f2d3..bf091cd 100644 --- a/Services/YouTubeAuthService.cs +++ b/Services/YouTubeAuthService.cs @@ -15,7 +15,25 @@ public class YouTubeAuthService { private const string AuthorizationEndpoint = "https://accounts.google.com/o/oauth2/v2/auth"; private const string TokenEndpoint = "https://oauth2.googleapis.com/token"; - private const string Scope = "https://www.googleapis.com/auth/youtube https://www.googleapis.com/auth/youtube.force-ssl"; + /// + /// The single scope the app requests: auth/youtube ("Manage your YouTube + /// account"). It is the **broadest** YouTube scope and is unavoidable — no narrower one + /// covers live broadcast create/transition, and Google's own liveBroadcasts.insert + /// reference lists it as an accepted alternative to youtube.force-ssl, so every + /// endpoint this app touches (channels, liveBroadcasts, liveStreams, + /// liveChat, playlistItems) is covered by it. + /// + /// auth/youtube.force-ssl used to be requested alongside it and was + /// dropped 2026-09-28 as redundant: its grant is "see, edit, and permanently delete + /// your YouTube videos, ratings, comments and captions", a strict subset of what + /// auth/youtube already grants. Requesting both meant two consent lines and two + /// scopes to justify for one capability. Removing a scope can never narrow capability, + /// because the scope being kept is the broader of the two. + /// + /// No forced re-consent: a refresh token keeps the scopes it was originally granted, so an + /// existing session is unaffected; the reduced set applies from the next fresh sign-in. + /// + private const string Scope = "https://www.googleapis.com/auth/youtube"; private readonly string _clientId; private readonly string _clientSecret; diff --git a/ytLive.Tests/YouTubeAuthServiceTests.cs b/ytLive.Tests/YouTubeAuthServiceTests.cs index 7258340..899e72f 100644 --- a/ytLive.Tests/YouTubeAuthServiceTests.cs +++ b/ytLive.Tests/YouTubeAuthServiceTests.cs @@ -1,5 +1,8 @@ +using System.IO; +using System.Linq; using System.Net; using System.Text; +using System.Text.RegularExpressions; using Xunit; using ytLive.Models; using ytLive.Services; @@ -117,4 +120,45 @@ public class YouTubeAuthServiceTests // The last attempt is deliberately left pending — nothing can cancel it and the // test host exits before its 5-minute timer fires. } + + /// + /// The app requests exactly ONE scope, and it is the broad one. 2026-09-28: + /// `auth/youtube.force-ssl` was dropped as redundant — its grant ("see, edit, and + /// permanently delete your videos, ratings, comments and captions") is a strict subset of + /// `auth/youtube` ("Manage your YouTube account"), which is also the only scope that can + /// cover live broadcast create/transition, so it was never avoidable. + /// + /// Asserted from source because the const is private and the auth flow cannot be exercised + /// without a browser. What is worth locking in is the DECISION — that the redundant scope + /// does not come back — not merely today's string. + /// + /// The assertion reads the const's value out of the source rather than scanning the + /// file. A file-wide `DoesNotContain("force-ssl")` fails on this very file's doc comment, + /// which names force-ssl precisely to explain why it went. A test that fails on the prose + /// documenting the guarantee gets deleted instead of obeyed. + /// + [Fact] + public void ExactlyOneScopeIsRequested_AndItIsTheBroadOne() + { + var source = File.ReadAllText(RepoFile("Services", "YouTubeAuthService.cs")); + + var match = Regex.Match(source, @"private const string Scope = ""([^""]*)"";"); + Assert.True(match.Success, "could not find the Scope const in YouTubeAuthService.cs"); + + var scopes = match.Groups[1].Value.Split(' ', StringSplitOptions.RemoveEmptyEntries); + Assert.Equal(new[] { "https://www.googleapis.com/auth/youtube" }, scopes); + } + + private static string RepoFile(params string[] parts) + { + var dir = new DirectoryInfo(AppContext.BaseDirectory); + while (dir != null) + { + var candidate = Path.Combine(new[] { dir.FullName }.Concat(parts).ToArray()); + if (File.Exists(candidate)) return candidate; + dir = dir.Parent; + } + throw new FileNotFoundException( + $"could not locate {Path.Combine(parts)} above {AppContext.BaseDirectory}"); + } }