gramps/LlamaCasty
0
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Drop the redundant force-ssl scope, and reject the playlistItems "fix"

Browse Source 
force-ssl is gone from the requested scope. Its grant ("see, edit, and
permanently delete your videos, ratings, comments and captions") is a strict
subset of auth/youtube's ("Manage your YouTube account"), and 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 by the one we keep. Two
consent lines became one, and one fewer scope to justify. Removing a scope
cannot narrow capability when the scope being kept is the broader of the two,
and there is no forced re-consent: a refresh token keeps the scopes it was
originally granted, so existing sessions are untouched.

The playlistItems change is NOT being made, and the map entry recommending it
was wrong. It came out of the verification hunt on the theory that public data
should not ride on user credentials. Walking the code kills it: ChannelStats-
Service is built with one HttpClient, and channels?mine=true on the same
service genuinely needs the session. Splitting it means a second unauthenticated
client PLUS a Google API key, since every YouTube Data API request needs a
credential and the call 403s without one. So the fix would ship a new
extractable, quota-abusable credential inside the binary to replace a token the
app already holds, buying zero verification-surface reduction (the data is
public, no scope involved) and zero correctness. Strictly worse. Recorded as a
do-not-do so it is not "helpfully" reintroduced later.

Test: ExactlyOneScopeIsRequested_AndItIsTheBroadOne asserts on the const's
parsed VALUE rather than scanning the file — a file-wide DoesNotContain
fails on the doc comment that names force-ssl to explain its removal, which is
the same trap I hit and documented earlier today in the locator test. Suite
380/380, 0 warnings.
This commit is contained in:
gramps
2026-09-28 09:45:35 -07:00
parent 4fcf76289c
commit b3edc2e42e
4 changed files with 110 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
HANDOFF.md
+21 -12
View File
@@ -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**
MyMistakes.md
+26
View File
@@ -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.
Services/YouTubeAuthService.cs
+19 -1
View File
@@ -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";
/// <summary>
/// The single scope the app requests: <c>auth/youtube</c> ("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 <c>liveBroadcasts.insert</c>
/// reference lists it as an accepted alternative to <c>youtube.force-ssl</c>, so every
/// endpoint this app touches (<c>channels</c>, <c>liveBroadcasts</c>, <c>liveStreams</c>,
/// <c>liveChat</c>, <c>playlistItems</c>) is covered by it.
///
/// <para><c>auth/youtube.force-ssl</c> used to be requested alongside it and was
/// <b>dropped 2026-09-28 as redundant</b>: its grant is "see, edit, and permanently delete
/// your YouTube videos, ratings, comments and captions", a strict subset of what
/// <c>auth/youtube</c> 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.</para>
/// </summary>
private const string Scope = "https://www.googleapis.com/auth/youtube";
private readonly string _clientId;
private readonly string _clientSecret;
ytLive.Tests/YouTubeAuthServiceTests.cs
+44
View File
@@ -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.
}
/// <summary>
/// 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 <b>value</b> 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.
/// </summary>
[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}");
}
}