diff --git a/CHANGELOG.md b/CHANGELOG.md index 38d7427e..5f4e46e5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,12 @@ its first tagged release. ## [0.7.0] — unreleased +### Upgrading +- **TLS now serves on :443 rather than :8080 when a certificate is configured.** + An install reached at `https://host:8080` moves, and a firewall that only + opens 8080 makes the console unreachable after the upgrade with nothing on + screen to say why. Open 443, or set the listen address back explicitly. + ### Fixed - **The dashboard's grouped destination list and the Prometheus scrape lost every programme but one.** Scoping `Engine.Status` to its own source was diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4d76e5b3..cbafdfa3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -39,7 +39,7 @@ make build # builds the UI, embeds it, produces ./polyemesis ./polyemesis ``` -You need Go 1.26.5+, Node 20.19+ or 22.12+ (Vite 8's floor; CI builds on 24), +You need Go 1.27.0+, Node 20.19+ or 22.12+ (Vite 8's floor; CI builds on 24), and FFmpeg 6.0+ — 8.x recommended. See [docs/INSTALL.md](docs/INSTALL.md) for platform detail and [docs/DEPENDENCIES.md](docs/DEPENDENCIES.md) for what is pinned and why. diff --git a/config.example.yaml b/config.example.yaml index da1934c3..f158b898 100644 --- a/config.example.yaml +++ b/config.example.yaml @@ -112,3 +112,11 @@ ffmpeg: # that unknown keys are ignored, so it had been offering a switch that did # nothing. A config file that still carries it keeps loading. For multitrack # ingest today, use SRT. + +# The optional whisper.cpp CLI, used by the transcription jobs. Every field is +# optional: leave the whole block out and polyemesis searches $PATH for +# `whisper-cli` (or the older `main`), which is what a package-manager install +# puts there. Set it only when the binary lives somewhere $PATH does not reach +# -- a build in /opt, or a container that mounts it late. +transcription: + binary: "" diff --git a/docs/DESIGN-OAUTH-BROKER.md b/docs/DESIGN-OAUTH-BROKER.md new file mode 100644 index 00000000..f26c370f --- /dev/null +++ b/docs/DESIGN-OAUTH-BROKER.md @@ -0,0 +1,184 @@ +# Connecting a platform without registering an application + +**Status: a decision recorded, not a change made.** Nothing in this file is +built. It exists so that whoever builds it starts from what was researched on +2026-08-26 rather than from an assumption about how OBS works. + +## The problem, stated as the operator's experience + +To connect one platform today, an operator registers a developer application on +that platform, copies a client ID and a client secret into polyemesis, and sets +a redirect URI that must match exactly. For four platforms that is four +dashboards, four sets of credentials, and — for one of them — a review process +measured in days. + +Every self-hoster does this individually. The work is identical every time and +none of it is about their broadcast. + +The friction is also **wildly uneven**, which matters when choosing what to fix: + +| Platform | What the operator faces | +|---|---| +| Twitch | Register app, set redirect, copy two values. Minutes | +| Kick | Same, plus one trap: the stream key is withheld unless `streamkey:read` is granted, and an account connected before that scope was requested must be disconnected and reconnected | +| YouTube | Google Cloud project, enable YouTube Data API v3, consent screen, add yourself as a **test user**. More clicks, no review | +| Facebook | Create app, add Facebook Login, and then **App Review for Advanced Access** if anyone other than the app's own developer will connect. Days | + +## How OBS solves it, read from its source on 2026-08-26 + +OBS is the closest comparable: a desktop application, widely forked and +repackaged, that offers one-click sign-in to Twitch, YouTube and Restream. + +**It runs a broker.** From +[`frontend/oauth/TwitchAuth.cpp`](https://github.com/obsproject/obs-studio/blob/master/frontend/oauth/TwitchAuth.cpp): + +```c +#define TWITCH_AUTH_URL OAUTH_BASE_URL "v1/twitch/redirect" +#define TWITCH_TOKEN_URL OAUTH_BASE_URL "v1/twitch/token" +``` + +and from `frontend/CMakeLists.txt`: + +```cmake +if(NOT OAUTH_BASE_URL) + set(OAUTH_BASE_URL "https://auth.obsproject.com/" CACHE STRING "Default OAuth base URL") +``` + +OBS does not call Twitch's token endpoint. The exchange goes through a service +the OBS project operates. The client **ID** is compiled in and lightly +obfuscated (`deobfuscate_str`, `frontend/utility/obf.h`); the client **secret** +is not in the binary at all, because the broker holds it. + +**YouTube is the exception, and the exception is instructive.** +`frontend/cmake/templates/ui-config.h.in` defines: + +```c +#define TWITCH_CLIENTID "@TWITCH_CLIENTID@" +#define YOUTUBE_CLIENTID "@YOUTUBE_CLIENTID@" +#define YOUTUBE_SECRET "@YOUTUBE_SECRET@" +``` + +Twitch ships an ID only; YouTube ships an ID *and* a secret. Google's installed- +application model treats that secret as non-confidential — it is a public client +in the RFC 8252 sense — while Twitch requires a confidential client. **The +platform's own rules decide whether a broker is needed at all**, and they differ +per platform. Any design here that treats "OAuth" as one thing will be wrong for +at least one of the four. + +**A build without credentials loses the feature**, from +`frontend/cmake/feature-twitch.cmake`: + +```cmake +if(TWITCH_CLIENTID AND TWITCH_HASH MATCHES "^(0|[a-fA-F0-9]+)$" AND TARGET OBS::browser-panels) + target_enable_feature(obs-studio "Twitch API connection" TWITCH_ENABLED) +else() + target_disable_feature(obs-studio "Twitch API connection") +``` + +So a distribution rebuilding OBS from source ships an OBS with no Twitch +integration. That is the honest cost of the model and OBS accepts it. **We would +be accepting it too**, and for a project whose users build from source more often +than OBS's do, it lands harder. + +## The options + +| | Operator effort | What it costs the project | +|---|---|---| +| **Today** — every operator registers their own apps | High, and repeated per platform | Nothing. No service, no secrets held, no availability promise | +| **Broker**, OBS's model | Near zero | A service to run, secure and keep up. It sees every token in transit. Every install depends on its uptime | +| **Ship a client ID, no secret** | Zero, where the platform permits a public client | Only lawful where PKCE-without-secret is allowed. Already true for Kick | +| **Hybrid** — broker by default, bring-your-own always supported | Zero by default, full control when wanted | Two code paths, and both must be tested | + +## What is recommended, and the reasoning + +**The hybrid, defaulting to bring-your-own credentials.** + +The argument against a mandatory broker is not operational, it is what the +product claims to be. `README.md` sells a single static binary with no runtime +dependencies, and `docs/COMPARISON.md` sells self-hosting as the thing that +distinguishes polyemesis from restream.io. A required call to a server we operate +makes every self-hosted install depend on our uptime and routes every operator's +tokens through us. That is a change to the promise, not a convenience feature, +and it should never be the only path. + +As an **option**, it is straightforwardly good: the operator who wants one click +gets one click, and the operator who wants nothing to phone home keeps that. + +### Two cheaper wins to take first + +1. **Kick needs no broker at all.** It is OAuth 2.1 with PKCE — a public client, + no secret. A client ID could ship today and one of the four platforms becomes + one-click with no infrastructure and no secret held anywhere. This is the + highest ratio of benefit to risk on the list and it is available now. + +2. **Facebook cannot be fixed by any of this.** App Review gates the + *permissions* — `publish_video`, `pages_manage_posts` — not the credentials. + A broker using our client ID would still require every operator's use to be + covered by *our* App Review, which is a materially different and larger + commitment than relaying a token. Facebook stays bring-your-own. + +So the realistic end state is: **Kick free, Twitch and YouTube one-click via the +broker, Facebook unchanged.** Anyone estimating this work should price it as +"two and a half platforms", not four. + +## The broker, if it is built + +Cloudflare Workers fits, and the shape is small: two routes, four secrets, no +database. + +| Need | Mechanism | +|---|---| +| Hold the client secrets | Workers Secrets (encrypted bindings, read from `env`) | +| `/v1//redirect` and `/v1//token` | one Worker, path-routed | +| Call the platform's token endpoint | one `fetch()` subrequest | +| Stable redirect URI | custom domain on a Worker route | +| Cost | free plan allows 100,000 requests/day | + +Traffic is proportional to **account connections**, not to streaming, so the free +tier is not a constraint in any realistic deployment. + +Two platform limits are worth writing down before someone meets them: + +* The free plan allows **50 external subrequests per invocation**. A token + exchange is one. Not a constraint. +* Exceeding the daily request limit returns **Error 1027**, and the route's fail + mode is configurable. This must be **fail closed**. Fail open bypasses the + Worker, and for a security-critical route that is worse than refusing. + +### The broker must be stateless, and that is the point + +The obvious implementation stores the PKCE verifier and state in KV for the few +seconds between redirect and callback. **Do not.** Sign the state parameter with +a Worker secret and carry what is needed inside it. + +This removes KV, removes expiry sweeping, and removes the only reason the broker +would have a datastore. What it buys is a sentence that can be written in the +documentation and be true: **the broker stores nothing about anyone.** + +For a product whose distinguishing claim is that nothing phones home, the +difference between "a service that holds your data" and "a service that relays +one exchange and forgets" is worth more than the engineering it saves. + +**It still sees every token in transit.** That is unavoidable in this design and +must be stated plainly wherever the broker is offered, not buried. An operator +who objects has the bring-your-own path, which is exactly why that path stays. + +## What would have to be true before building it + +* The broker's client IDs are registered to the **project**, which means the + project accepts each platform's developer terms on behalf of every operator + who uses the default path. Read them first; at least one will have something + to say about it. +* A rate limit per operator, or one abusive install exhausts a shared quota for + everyone. Trovo, for reference, publishes 1200 requests/minute **per client + id** — shared across every user of that id. +* An answer to "the broker is down and I cannot connect an account", which is + bring-your-own, and that answer only exists because the hybrid keeps it. +* A decision on what happens to tokens already issued through the broker if the + broker is ever retired. + +## The rule this leaves + +**Bring-your-own credentials must always work, and must always be documented as +the path that depends on nothing.** Anything built on top of that is a +convenience, and a convenience may be switched off. diff --git a/docs/PLATFORMS.md b/docs/PLATFORMS.md index 44d2f81c..fdcd1230 100644 --- a/docs/PLATFORMS.md +++ b/docs/PLATFORMS.md @@ -27,7 +27,7 @@ below. The table is what each platform's **published API** allows today. The same matrix is rendered in `Settings → Platform credentials` and served from `GET /api/v1/platforms/capabilities`. -| Platform | Sign in | Stream key | Metadata | Chat read | Chat send | Moderation | Viewers Start / end | +| Platform | Sign in | Stream key | Metadata | Chat read | Chat send | Moderation | Viewers | Start / end | |---|---|---|---|---|---|---|---|---| | **YouTube Live** | Works | Works | Works | Works | Works | Works | Works | Works | | **Twitch** | Works | Works | Works | Works | Works | Works | Works | Not possible | diff --git a/docs/evidence/vimeo-trovo-oauth-2026-08-26.md b/docs/evidence/vimeo-trovo-oauth-2026-08-26.md new file mode 100644 index 00000000..9b909abe --- /dev/null +++ b/docs/evidence/vimeo-trovo-oauth-2026-08-26.md @@ -0,0 +1,165 @@ +# Vimeo and Trovo OAuth and live APIs, checked 2026-08-26 + +Researched because both appear in the destination preset catalogue +(`internal/db/platforms.go`) and `docs/PLATFORMS.md` records them as +*unverified* — "not built, and the platform's API not confirmed either way". +This file confirms them either way. It does not propose building either. + +Same standard as `platform-lifecycle-apis-2026-08-16.md`: every claim traces to +the platform's own reference page, read on the date in the title, with the +operative sentence quoted verbatim. A capability with no dated source is not +recorded. + +## The fetch trap, again, in a new shape + +The 2026-08-16 pass recorded two hosts that answer **HTTP 200 with a +"page not found" body**. A third shape was hit here and is worth adding: + +* `developer.vimeo.com` answers **HTTP 200 with a 0-byte body containing only + the word "Vimeo"** to a non-browser fetch. The documentation is rendered + client-side. A fetcher that trusted the status code would have concluded the + page was empty, and a fetcher that trusted its own memory would have filled + the gap from training data — which is the failure this file format exists to + prevent. Every Vimeo claim below was read from a **rendered** page. + +`developer.trovo.live` serves static HTML and needed no browser. + +--- + +## The finding that decides Vimeo + +> "Please note that our live API is available only to Vimeo Enterprise +> customers." +> +> — [Vimeo API Reference: Live](https://developer.vimeo.com/api/reference/live), +> read 2026-08-26, rendered + +**Vimeo's OAuth is open to any app; Vimeo's LIVE API is not.** The distinction +matters more than the presence of a token endpoint: an operator can authenticate +and still be unable to create an event. For a self-hosted product whose users +are individuals and small teams, that is the same practical wall as LinkedIn +Live's partner gate — the flow works and the capability is unreachable. + +Also recorded, because it changes what anyone building this should target: + +> "One-time live events are being deprecated. We recommend that you avoid using +> the methods for these." +> +> — same page, read 2026-08-26 + +### Vimeo OAuth, which is genuinely open + +| item | value | source | +|---|---|---| +| authorize | `https://api.vimeo.com/oauth/authorize?response_type=code&client_id={client_id}&redirect_uri={redirect_uri}&state={state}&scope={scope_list}` | [Working with Authentication](https://developer.vimeo.com/api/authentication), read 2026-08-26 | +| token exchange | `https://api.vimeo.com/oauth/access_token` | same | +| device flow | `https://api.vimeo.com/oauth/device`, `https://api.vimeo.com/oauth/device/authorize` | same | +| grant types | client credentials, authorization code, implicit, **device code** | same, Table 2 | +| scopes | `public`, `private`, `purchased`, `create`, `edit`, and others in Table 1 | same, Table 1 | + +Device code is present, which is the flow polyemesis already implements in +`internal/oauth/device.go`. Had the live API been reachable, this would have been +the cheapest of the two to add. + +**PKCE: not established.** The authentication page's grant-type table does not +mention it and no PKCE parameter was seen. Recorded as *unknown*, not as absent. + +### Vimeo live capabilities, for the record + +Behind the Enterprise gate, the surface is complete: create / update / delete an +event, **activate an event**, **end an event**, get ingest status, event +destinations (including RTMP), M3U8 playback, thumbnails, speakers, audio-track +settings and viewer analytics export. So the lifecycle polyemesis models — +create, go live, end — maps cleanly. The obstacle is commercial, not technical. + +--- + +## Trovo: fully documented and open + +Every row below is from +[Trovo APIs & OAuth Developer Doc](https://developer.trovo.live/docs/APIs.html), +read 2026-08-26. + +| capability | verdict | endpoint | scope | +|---|---|---|---| +| authorize (code flow) | documented | `https://open.trovo.live/page/login.html?client_id=…&response_type=code&scope=…&redirect_uri=…&state=…` | n/a | +| authorize (implicit) | documented | same URL, `response_type=token` | n/a | +| validate token | documented | `GET https://open-api.trovo.live/openplatform/validate` | **None** | +| revoke token | documented | §4.2 | — | +| refresh token | documented | `POST https://open-api.trovo.live/openplatform/refreshtoken` | — | +| **stream key** | documented | `GET https://open-api.trovo.live/openplatform/channel` | `channel_details_self` | +| metadata (title, category, language, audience) | documented | `POST https://open-api.trovo.live/openplatform/channels/update` | `channel_update_self` | +| chat read | documented | websocket, see [Chat Service](https://developer.trovo.live/docs/Chat%20Service.html) | — | +| chat send | documented | `POST https://open-api.trovo.live/openplatform/chat/send` | `chat_send_self` + `send_to_my_channel` | +| moderation (ban, mod, delete) | documented | `POST https://open-api.trovo.live/openplatform/channels/command` | `manage_messages` | +| viewer count | documented | `POST https://open-api.trovo.live/openplatform/channels/{channel_id}/viewers` | **None**, and no access token | +| broadcast start / end | **not possible** | no such endpoint exists in the reference | — | + +### The full scope list, verbatim + +| Scope | Description | +|---|---| +| `user_details_self` | View your email address and user profiles. | +| `channel_details_self` | View your channel details. Including Stream Key. | +| `channel_update_self` | Update your channel settings | +| `channel_subscriptions` | Get your subscribers list. | +| `chat_send_self` | Send chat messages on behalf of myself. | +| `send_to_my_channel` | Send chat messages to my channel. | +| `manage_messages` | Perform chat commands and delete chat messages. | + +### Refresh, which is the half that matters at hour four + +> "A refresh token can hold a maximum of 50 access tokens at the same time. If +> exceeded, you should wait for the old access tokens to expire before you can +> refresh again." + +> "the new segment "refresh_token" is added in the response. This means you can +> always get a new refresh_token with 30 days lifetime from your old effective +> refresh_token before it's going to expire." + +Access tokens expire in `14400` seconds — four hours, per the response sample. +Refresh tokens last 30 days and are rotated on use, and the old one keeps working +until it expires. **The fifty-token ceiling is the trap**: a client that refreshes +on a timer rather than on expiry would exhaust it, and the failure arrives as a +refused refresh rather than as a rejected request. + +### Rate limit — a documented number, which is unusual + +> "When an application is first registered, your application will get a rate +> limit of 1200 requests per minute." + +Headers `x-ratelimit-limit`, `x-ratelimit-remaining`, `x-ratelimit-reset`, and +status `11706` with message "API rate limit exceeded" when exhausted. + +The 2026-08-16 file's rule was that a numeric limit the docs refuse to state must +stay unstated in code. Trovo states one, so it may be relied on — but the header +is still the authority, because the doc also says a higher limit can be granted +per client id. + +--- + +## What is NOT recorded here + +* **Trovo's code→token exchange endpoint.** §3.2 describes the authorize step and + the reference documents validate, revoke and refresh. The exchange call itself + was not captured verbatim in this pass. Anyone implementing must read §3.2 step + 2 and record it before writing the call — do not infer it from the refresh + endpoint's shape. +* **Whether Trovo supports PKCE.** Not mentioned. The documented code flow uses a + `client_secret`, which for polyemesis means a confidential client. +* **Vimeo Enterprise pricing or whether a trial exposes the live API.** Not a + documentation question. + +## What this changes today + +Nothing. `docs/PLATFORMS.md` records both as *unverified*, and on this evidence: + +* **Trovo** would move to buildable — seven capabilities documented with scopes, + and it maps onto polyemesis's existing matrix better than any unbuilt platform + examined so far. Start/end is genuinely *not possible*, which matches Twitch + and Kick. +* **Vimeo** should be recorded as gated rather than unverified. The API exists, + is complete, and is unreachable without an Enterprise contract. + +Updating `PLATFORMS.md` is deliberately left to whoever decides to act on this; +a matrix cell that says "Works" must mean polyemesis does it today. diff --git a/internal/db/backup_key_survives_test.go b/internal/db/backup_key_survives_test.go new file mode 100644 index 00000000..22f8d3f0 --- /dev/null +++ b/internal/db/backup_key_survives_test.go @@ -0,0 +1,86 @@ +package db + +import ( + "path/filepath" + "testing" +) + +// Retyping ONE key must not destroy the other one's ciphertext. +// +// The recovery this protects is the ordinary one: a database restored without +// its secret.key. Every destination reads back with an empty StreamKey -- the +// fail-closed rule -- and is flagged KeyUnreadable. The sealed bytes are still +// there, and putting the right key file back returns every destination. +// +// The guard that keeps those bytes required BOTH halves to be empty. So an +// operator who gave up on the primary and retyped it took the re-sealing branch +// for both columns, and sealStreamKey("") returns nil bytes: the BACKUP's +// ciphertext became NULL. Recovering one half destroyed the other, silently, +// and no later secret.key could bring it back. +// +// Mutation: restore keepsSealedKey's `&&` form at both call sites in +// UpdateDestination. Observed to fail with the backup ciphertext gone. +func TestRetypingThePrimaryKeyLeavesTheBackupCiphertextAlone(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "polyemesis.db") + d := keyDB(t, path, WithSecretBox(testBox(t))) + + src := &Source{Name: "Main", Enabled: true, Ingest: DefaultSettings().Ingest} + if err := d.CreateSource(src); err != nil { + t.Fatalf("CreateSource: %v", err) + } + + saved, err2 := d.CreateDestination(&Destination{ + Name: "has both halves", Kind: DestRTMP, URL: "rtmp://example.invalid/live", + StreamKey: "primary-secret", BackupStreamKey: "backup-secret", + Enabled: false, AudioBitrate: 160, + }) + if err2 != nil { + t.Fatalf("CreateDestination: %v", err2) + } + + sealedBefore := rawCol(t, d, "backup_stream_key_enc", saved.ID) + if len(sealedBefore) == 0 { + t.Fatal("the backup key was not sealed at all, so this test cannot show it surviving") + } + + // The state after a restore with no key file: both halves read back empty + // and the row is flagged. The operator retypes the PRIMARY only. + row := *saved + row.KeyUnreadable = "the stored key could not be opened with this secret.key" + row.StreamKey = "primary-retyped" + row.BackupStreamKey = "" + if _, err := d.UpdateDestination(&row); err != nil { + t.Fatalf("UpdateDestination: %v", err) + } + + sealedAfter := rawCol(t, d, "backup_stream_key_enc", saved.ID) + if len(sealedAfter) == 0 { + t.Fatal("retyping the primary key destroyed the BACKUP key's ciphertext. " + + "That is the one thing a restored secret.key could still have recovered, " + + "and the operator was not asked and not told.") + } + if string(sealedAfter) != string(sealedBefore) { + t.Error("the backup ciphertext was rewritten rather than left alone; " + + "whatever it now holds was not sealed from the operator's backup key") + } + + // The control: the half the operator DID retype must actually change, or + // this test would pass on a write that did nothing at all. + if got, err := d.GetDestination(saved.ID); err != nil { + t.Fatalf("GetDestination: %v", err) + } else if got.StreamKey != "primary-retyped" { + t.Errorf("StreamKey = %q, want the retyped value -- the write was skipped "+ + "entirely, so the assertion above proves nothing", got.StreamKey) + } +} + +func rawCol(t *testing.T, d *DB, col string, id int64) []byte { + t.Helper() + var b []byte + // A literal column name from this file, not from a caller. + if err := d.sql.QueryRow(`SELECT `+col+` FROM destinations WHERE id=?`, id).Scan(&b); err != nil { + t.Fatalf("read %s: %v", col, err) + } + return b +} diff --git a/internal/db/destinations.go b/internal/db/destinations.go index df30b194..0ba8ada7 100644 --- a/internal/db/destinations.go +++ b/internal/db/destinations.go @@ -1039,7 +1039,11 @@ const ( const ( destUpdateKeyCols = `stream_key=?, stream_key_enc=?, backup_stream_key=?, backup_stream_key_enc=?, ` - destUpdateCols = `name=?, kind=?, platform=?, account_id=?, url=?, + // The two halves are re-sealed independently, because an operator who + // retypes one is not saying anything about the other. See keepsSealedKey. + destUpdatePrimaryKeyCols = `stream_key=?, stream_key_enc=?, ` + destUpdateBackupKeyCols = `backup_stream_key=?, backup_stream_key_enc=?, ` + destUpdateCols = `name=?, kind=?, platform=?, account_id=?, url=?, backup_url=?, backup_ingest_wanted=?, enabled=?, audio_bitrate=?, profile=?, rendition_id=?, source_id=?, extra_input_args=?, extra_output_args=?, expert_ack_reencode=?, @@ -1051,6 +1055,9 @@ const ( updated_at=? WHERE id=?` destUpdateQuery = `UPDATE destinations SET ` + destUpdateKeyCols + destUpdateCols destUpdateKeepKeyQuery = `UPDATE destinations SET ` + destUpdateCols + // Keep one sealed column and re-seal the other. + destUpdateKeepBackupQuery = `UPDATE destinations SET ` + destUpdatePrimaryKeyCols + destUpdateCols + destUpdateKeepPrimaryQuery = `UPDATE destinations SET ` + destUpdateBackupKeyCols + destUpdateCols ) // keepsSealedKey reports whether a write must leave the stored key columns @@ -1076,8 +1083,29 @@ const ( // destinations that are actually in that state -- a client cannot use it to // pin a key it does not know, because with no key in the body there is no // value for the write to have carried anyway. +// PER HALF, because the halves fail independently and an operator who retypes +// one has said nothing about the other. +// +// Requiring BOTH to be empty meant that supplying the primary key on a +// destination that also had a backup took the re-sealing branch for both +// columns -- and sealStreamKey("") returns nil bytes, so backup_stream_key_enc +// became NULL. The ciphertext that putting the right secret.key back would have +// recovered was destroyed by the act of recovering the other half, and nothing +// was said. The read path condemns both together, which is right: neither can +// be shown to be readable. The write path inherited that coupling, where it is +// wrong. +func keepsSealedPrimaryKey(dst *Destination) bool { + return dst.KeyUnreadable != "" && dst.StreamKey == "" +} + +func keepsSealedBackupKey(dst *Destination) bool { + return dst.KeyUnreadable != "" && dst.BackupStreamKey == "" +} + +// keepsSealedKey is both halves: the ordinary case, where a rename carried no +// key at all. func keepsSealedKey(dst *Destination) bool { - return dst.KeyUnreadable != "" && dst.StreamKey == "" && dst.BackupStreamKey == "" + return keepsSealedPrimaryKey(dst) && keepsSealedBackupKey(dst) } // checkRendition rejects a rendition_id that names no rendition. The foreign @@ -1309,10 +1337,28 @@ func (d *DB) UpdateDestination(dst *Destination) (*Destination, error) { dst.Multitrack, vodProfile, time.Now().Unix(), dst.ID, } - query := destUpdateQuery - if keepsSealedKey(dst) { + // Four cases, because the two halves are decided separately. + keepPrimary, keepBackup := keepsSealedPrimaryKey(dst), keepsSealedBackupKey(dst) + var query string + switch { + case keepPrimary && keepBackup: query = destUpdateKeepKeyQuery - } else { + case keepPrimary: + query = destUpdateKeepPrimaryQuery + backupEnc, backupPlain, err := d.sealStreamKey(dst.BackupStreamKey) + if err != nil { + return nil, fmt.Errorf("seal backup stream key: %w", err) + } + args = append([]any{backupPlain, backupEnc}, args...) + case keepBackup: + query = destUpdateKeepBackupQuery + keyEnc, keyPlain, err := d.sealStreamKey(dst.StreamKey) + if err != nil { + return nil, fmt.Errorf("seal stream key: %w", err) + } + args = append([]any{keyPlain, keyEnc}, args...) + default: + query = destUpdateQuery keyEnc, keyPlain, err := d.sealStreamKey(dst.StreamKey) if err != nil { return nil, fmt.Errorf("seal stream key: %w", err) diff --git a/internal/oauth/platforms_doc_drift_test.go b/internal/oauth/platforms_doc_drift_test.go index 8241a226..c4d30af0 100644 --- a/internal/oauth/platforms_doc_drift_test.go +++ b/internal/oauth/platforms_doc_drift_test.go @@ -53,6 +53,32 @@ func TestPlatformsDocMatrixMatchesTheCapabilityMatrix(t *testing.T) { CapBroadcastLifecycle, } + // THE HEADER ROW, which nothing checked until a column went missing from it. + // + // The row regexp below matches DATA rows, and the count check tells the + // capability table apart from the others -- so a header that had lost a `|` + // still parsed, and every data row still had the right number of cells. The + // document rendered with "Viewers" and "Start / end" fused into one heading + // and every column after it labelled with its neighbour's name, while this + // test stayed green. + // + // Comparing the header's CELL COUNT to cols is the whole fix: it is the one + // thing that ties what the reader sees at the top of the table to what this + // test compares underneath it. + hdrRe := regexp.MustCompile(`(?m)^\| *Platform *\|.+\| *$`) + hdr := hdrRe.FindString(string(raw)) + if hdr == "" { + t.Fatal("no `| Platform | ...` header row in PLATFORMS.md; the capability " + + "table has moved or been renamed, and this test is comparing nothing") + } + hdrCells := strings.Split(strings.TrimSuffix(strings.TrimPrefix(strings.TrimSpace(hdr), "|"), "|"), "|") + if got, want := len(hdrCells)-1, len(cols); got != want { + t.Errorf("the capability table's header has %d capability columns, want %d.\n %s\n\n"+ + "A header that has lost a `|` still parses and every data row still has the "+ + "right cell count, so the page renders with two headings fused and every "+ + "column after them labelled with its neighbour's name.", got, want, strings.TrimSpace(hdr)) + } + rowRe := regexp.MustCompile(`(?m)^\|\s*\*\*([^*]+)\*\*\s*\|(.+)\|\s*$`) rows := map[string][]string{} for _, m := range rowRe.FindAllStringSubmatch(string(raw), -1) { diff --git a/web/src/data/docs.mjs b/web/src/data/docs.mjs index efaa0896..f6b5af66 100644 --- a/web/src/data/docs.mjs +++ b/web/src/data/docs.mjs @@ -276,6 +276,7 @@ const WITHHELD_ROWS = [ ["DESIGN-DESTINATION-HEALTH.md", "Design note for unshipped work."], ["DESIGN-ONE-PORT-ONLY.md", "Design note for unshipped work."], ["DESIGN-ONE-PORT-INGEST.md", "Design note for unshipped work."], + ["DESIGN-OAUTH-BROKER.md", "Design note for unshipped work."], ["README.md", "A directory of the others; /docs is that page here."], ];