mirror of
https://github.com/screentinker/screentinker.git
synced 2026-08-13 22:03:13 -06:00
229 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
screentinker
|
6aeb703efe
|
Merge pull request #254 from ChrisChrome/main
Add org-level widget sandbox toggle. |
||
|
ScreenTinker
|
ec450929ce |
Escape user-controlled data at the HTML sinks it actually reaches
A QA sweep found unescaped interpolations outside the SSO work. Auditing them properly
turned up 34 genuine HTML sinks; 23 carry data a user, a device or an identity provider
controls, and those are escaped here.
The ones that mattered:
- app.js renders `user.name` in the shell on EVERY page, and an identity provider's
`name` claim is stored verbatim, so an IdP could script the whole dashboard
- designer element `label`/`location` and widget `location`/`query` land inside
value="" attributes, where a single quote breaks out
- content `folder` lands in a data-folder="" attribute
- device `name` is set by the operator OR reported by the panel itself
- workspace-members renders a SERVER error string through t(), which interpolates raw
⚠️ My first attempt was a codemod over everything my scanner flagged, and it was wrong.
It wrapped `progressText.textContent`, `block.title` and `confirm(...)` — none of which
are HTML, so escaping there shows users literal `<`. Worse, it wrapped
`title: ev.title ? ... : null`, an API PAYLOAD, which would have written escaped markup
into the database. I reverted the whole thing and narrowed to interpolations that are
genuinely inside an HTML template, then read all 34 and chose 23.
Skipped deliberately: static app strings, i18n output, ternaries yielding `selected`,
`window.location.origin`, and sites already escaped.
Verified in Chrome, not by inspection: the payload was seeded into user.name,
device.name, content.filename/folder, widget.name/config and video_wall.name (the first
attempt's seeds silently failed on column names — the API responses are checked now),
then eleven views were loaded. Zero executions, zero live img tags — AND the payload is
visible as inert text in 6/6 views, which is what proves the views rendered it rather
than the test proving nothing.
1609 tests; every frontend module parses as an ES module.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
fbf55f842c |
Close the third QA round: limiter bypass, stored XSS, break-glass, org placement
Four HIGH findings. Two were mine, and one was a composition of two of my own fixes.
ONE EXTRA SLASH DEFEATED EVERY /api/auth LIMITER
`/api/auth//login` still reaches the login handler — Express normalises the mount
boundary for the router — but `app.use('/api/auth/login', rateLimit(...))` does not
match it, so the limiter never runs. A review got a real session after 60 unthrottled
password attempts. Same for //totp/verify (unlimited 6-digit brute force),
//forgot-password (unlimited reset mail to any address) and //sso/discover (the
customer-enumeration cap, gone). Fixing the limiter KEY could never help, because the
middleware was never invoked: the path is now collapsed to one canonical form before
routing. Pre-existing, and it falsified this file's own warning about walking past the
login limiter.
STORED XSS: I ESCAPED ONE COPY OF THE TABLE
My earlier fix patched views/admin.js line 357 and missed line 372 in the same
function — and missed views/settings.js entirely, which renders a SECOND copy of the
platform users table from the same endpoint, including the email in a raw text node.
The write path was `POST /api/admin/users`, whose EMAIL_RE barred only whitespace, so
an org or workspace admin (not a platform admin) could choose an address that executed
in the operator's session. Both tables escaped, both regexes tightened to reject markup
characters, verified against 11 address shapes.
I KILLED THE BREAK-GLASS WHILE CLOSING AN ORACLE
Hoisting the domain check above the account lookup — my fix for the enumeration oracle
— made `user.role !== 'platform_admin'` unreachable for enforced domains. On a
self-host the operator IS the org owner, and my would_lock_out_actor guard GUARANTEES
their address is inside the enforced set, so the recovery loop closed on itself:
approving a removal request needs a signed-in platform admin. Both properties hold now
by letting the operator through on a CORRECT PASSWORD only — every wrong answer is the
identical 403 whether the address exists, does not exist, or is theirs. Verified: 200 /
403 / 403 / 403.
Also fixed: enabling SSO-only locked out every password-holding member including the
admin who pressed the button (password refused by policy, SSO refused by
account_exists_local). An org provider now adopts a password account at a domain it has
PROVED by DNS when the org requires SSO — which is what a verified domain means, and
what every hosted identity product does.
SSO USERS WERE LANDING IN A PERSONAL ORG
The membership write added organization_members but no workspace_members, and
ensureDefaultOrgForUser looks at workspaces — so it minted each SSO user a private
organization and made it their current one. The customer's Members page read
"Members (1)" while their staff signed in successfully and were invisible.
ALSO: bcrypt on a NULL password_hash 500'd with a stack (and was an oracle for accounts
a provider deletion had returned to local); stranded_members was returned by the server
and discarded by the UI; a provider with zero domains was the one useless state with no
warning; two limiter shapes were missing (removal-request shared the garbage bucket —
an unauthenticated flood could deny the SSO break-glass path); doubled mail subject
prefixes; a DELETE that toasted "Saved"; a decided request left in the DOM with live
listeners; and a confirm dialog promising "immediately" when sessions already open
survive.
1609 tests, three clean runs. Limiter, break-glass, oracle parity and null-password all
verified against a running server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
94e1273ecd |
Fix: per-organization SSO was blocked by our own CSP and had never worked in a browser
THE HEADLINE FEATURE COULD NOT RUN.
"Continue with single sign-on" was a <form method="POST"> that redirected on to the
customer's identity provider. Chrome applies `form-action` across the WHOLE redirect
chain, and the dashboard sets `form-action 'self'`, so the hop to the provider was
aborted — silently. The user clicked and nothing happened: no navigation, no toast, no
spinner, a byte-identical page. Combined with SSO-only it was a total lockout: password
login answers 403 "use the single sign-on button", pointing at a button that cannot
work.
Every test I ran on this feature checked the button RENDERED. None clicked it.
The provider origins cannot be allowlisted — customers supply them at runtime. So the
page now fetches the destination and navigates itself; a script-initiated navigation is
not governed by form-action. The redirect answer is kept for a caller without
JavaScript, where the chain stays same-origin until the provider takes over. The slug
in the JSON is not a disclosure: following the old redirect put it in the address bar
and history anyway.
Verified in Chrome: the provider start endpoint is reached, zero CSP violations, zero
aborted requests — where before it was ERR_ABORTED plus a console violation.
STORED XSS IN THE PLATFORM ADMIN'S SESSION
admin.js interpolated user name, email and auth_provider into innerHTML unescaped, and
/register accepted an address whose local part was an img tag with an onerror handler —
no spaces, so it slipped the asserted-email check too. A reviewer registered
anonymously and got script execution on #/admin: the page operators are now emailed to.
Escaped, and registration refuses addresses that are not addresses. (The render bug
predates this branch; the reachability and the significance of that screen do not.)
ALSO
- the org SSO button is secondary while a password still works; two identical blue
buttons stacked sent people to their IdP by muscle memory after typing a password.
1609 tests, three clean runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
85febe05c0 |
Fix a login-page dead end, an enumeration oracle, and three boot/limiter defects
From the regression sweep. The first is a genuine regression against main.
A RATE-LIMITED DISCOVERY PERMANENTLY DEAD-ENDED THE LOGIN PAGE
lookupOrgSso checked that a body PARSED, not that the request succeeded — and a 429
body is valid JSON. So `data.sso` came back undefined, the single sign-on button was
hidden, the password box restored, and the domain recorded as answered: permanently,
for the life of the page. On an SSO-only domain that is the worst outcome available —
the password box then returns 403 and the button the user is told to use is not on the
screen. Discover is 10/min per IP and one person filling in the form costs up to four
calls, so a few colleagues behind one office address is enough. The comment above that
code already claimed to prevent exactly this; it only ever covered the 5xx case.
THE SSO-ONLY REFUSAL WAS AN ACCOUNT-EXISTENCE ORACLE
403 for an address that exists, 401 for one that does not — from an endpoint whose own
lockout returns 401 specifically to avoid that. The DOMAIN check now runs BEFORE the
account lookup, so both answer identically; whether a domain uses single sign-on is
already public through /sso/discover, so it reveals nothing new. The membership-level
refusal is deliberately downgraded to the generic 401, because a distinct answer there
would put the oracle back for exactly the accounts worth enumerating.
Verified: existing and invented addresses at an SSO-only domain both 403; and on an
instance with NO SSO configured, register/login/wrong-password/unknown-address are
201/200/401/401 — the hoisted check does not touch them.
BOOT PREFLIGHT
- a cold install ran `npm ci --omit=dev` unconditionally, so a first start on a
developer machine left `npm test` broken: same class of surprise as the prune this
file already warns about, through the other branch of the same if. Now production-
only.
- two servers starting together: the loser died with ENOTEMPTY even though the tree
was complete by then. It re-checks before failing.
- the opt-out accepted only '1', unlike every other boolean the server takes.
THE LIMITER FOLD, DONE PROPERLY
Unmatched paths under /api/organizations still minted a bucket each. My first fix was a
catch-all regex — which put every unknown path in ONE bucket WITH the real endpoints,
so flooding nonsense URLs exhausted the limit for /sso-only. That trades a bypass for a
denial of service. Folding is now by explicit shape: known endpoints keep their own
keys, everything else shares a bucket kept apart from all of them.
Verified: 120 unmatched paths give 60/60 (bypass closed), and after that flood
/sso-only, /sso and /sso/:id/test all still answer 401 rather than 429 (no starvation),
while 70 hits on one real endpoint do trip its own limit. The login trailing-slash
bypass stays closed.
1609 tests, three clean runs.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
355b7a2b86 |
SSO: build the operator approval screen, and close the last of the QA findings
The approval workflow had no front door. The notification email told the operator to "review it in ScreenTinker under Admin" and that screen did not exist — the only way to approve was curl, while the tenant sat locked out of their own product. Admin now leads with a removal-request section: who asked, for which organization, the reason they gave, what approving does, and Approve/Reject. It hides itself when the queue is empty. Approving is confirmed; rejecting is not, because rejecting only leaves the safe state. REGISTRATION BYPASSED SSO-ONLY AND SQUATTED ADDRESSES /register had no domain awareness: it issued a working session at an SSO-only domain, and the account then held that address forever, because an SSO login will not adopt a row that has a password. Registering ceo@acme.test before the real CEO's first login left the address dead in both directions with no self-service way out. Refused now, and "Create Account" is hidden on the login page for those domains — it was the only action left on the card, so the page was inviting the one thing that cannot work. THE NEW RATE LIMIT WAS DECORATIVE /api/organizations carries three caller-chosen segments, and only the OIDC slug was folded — so every request minted its own bucket. Measured: 120 calls with unique org ids produced ZERO 429s, unauthenticated, against the limit that exists to bound outbound discovery and live DNS. Now 60/60. The general problem was named in the previous commit's own comment and then not applied to the mount it added. XSS IN THE TOAST showToast built innerHTML from server strings, including ones that reflect input verbatim — a reviewer typed `<img src=x onerror=alert(1)>` as an issuer and got script execution in the admin's session. Escaped. ALSO - the org SSO button sat BETWEEN the "Password" label and its input, so the label described the button and the field had none; moved below the input, with a for= - the OR divider survived when the providers under it were hidden - provider action buttons were clipped off-screen at 375px with no way to scroll to them — "Remove" was unreachable; the row wraps now 1609 tests. Verified in real Chrome: 13/13 on the approval loop and the login states, including approving a request and watching password login re-open for that org. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
|
ScreenTinker
|
901e664591 |
Fix: the router discarded every SSO return, so single sign-on could never complete
THE CRITICAL ONE. The server ends every SSO login by redirecting to `#/login?sso=1`
(claim the session) or `#/login?sso_error=<code>` (say what went wrong). The router
compared the hash EXACTLY against '#/login' in three places, so an unauthenticated
browser — the only kind that ever arrives there — had the hash rewritten to a bare
'#/login' and the query was gone before the login view ran.
- a user who authenticated perfectly at their IdP landed back on a clean login page,
still signed out, with no message: /api/auth/sso/claim was never called
- all 16 error codes rendered SILENCE — not a raw key, not "undefined", nothing to
report or search for
- it took the pre-existing ?verified=1 email-verification toast with it
The comment above the reset-password exclusion describes this exact bug class and was
never extended to the login route. It is now, in all three places: the auth redirect,
the render dispatch, and the no-workspace guard.
Verified in real Chrome: 16/16 codes render a real sentence, and ?sso=1 now reaches
POST /api/auth/sso/claim.
Also, on a server with NO SSO configured, confirmed in the browser that the login page
is exactly what it was before any of this work: email, password, Sign In, Forgot
password, zero SSO buttons, no single sign-on wording, plain local login issues a
session, no page errors.
And fixes MY preflight, which pruned devDependencies as a side effect of BOOTING:
`npm install --omit=dev` reconciles the whole tree, so merely starting the server
deleted socket.io-client, puppeteer-core and js-yaml and broke `npm test`. A reviewer
watched it happen. It now installs only the named missing packages, with --no-save —
a boot-time repair that quietly removes packages is worse than the failure it fixes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
0e8ffa5444 |
SSO-only: an org may require its own identity provider, operator approves removal
Per-organization toggle. Enabling is the safe direction and an org admin does it
alone; turning it back off is a REQUEST that a platform admin has to approve, because
that is the direction that re-opens password sign-in — the direction a compromised
admin would take, and the one a customer will demand at their worst moment with the
IdP down.
- requires at least one VERIFIED domain, so nobody can lock a company out of a
domain they only typed, and an org cannot leave its own people with no way in
- the login page HIDES the password field for those domains rather than letting
someone type a password that will be refused and then go reset it
- the refusal is `sso_required`, distinguishable from a wrong password
- the approval email carries NO action link: a token that acts on its own turns
every forwarded copy into a way to switch off a customer's SSO. The decision is
made signed in as a platform admin.
INSTANCE PROVIDERS WERE A SIDE DOOR
Blocking passwords while leaving "Continue with Google" is not requiring single
sign-on, it is renaming the bypass — instance-wide providers are the operator's and
are NOT domain-confined, so one could assert an address at an SSO-only domain and walk
straight past the customer's MFA and deprovisioning. The callback now refuses any
provider other than that organization's own, and the page stops offering them.
Instance-wide stays the default everywhere else: an address whose domain has no org
SSO still gets local plus every configured instance provider. The org only overrides
for its own verified domains.
PLATFORM_ADMIN IS EXEMPT, DELIBERATELY
The operator approves turning this off. If the operator's own address sat at an
SSO-only domain and that IdP broke, nobody could sign in to approve anything and the
instance would be bricked. The exemption is the break-glass, and a test pins it as
source so it is not "tidied away" as a convenience.
BROWSER-FOUND
Hiding the password by hiding its .form-group also hid the organization SSO button,
which lives inside that same group — leaving a login page whose only action was
"Create Account". Only visible by looking at a screenshot. Hides the field now, not
the container.
Player untouched: this branch changes no device, WebSocket or provisioning file, and
the 358 device/player/socket/pairing tests pass.
1603 tests pass. Enforcement, the approval workflow and the login page verified in
real Chrome.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
481a44c15a |
SSO settings: show a verification outcome once, not twice
First real-browser pass over this feature. Chrome via puppeteer-core, driving the
actual settings page: login, the SSO card, and a real click on Verify.
The click path works — the loadSso fix holds, no ReferenceError, and a failure shows
the specific DNS answer ("no _screentinker-verify record found ... DNS can take a few
minutes") rather than the generic catch-all. But the outcome was rendered TWICE: the
server persists last_error on the row and the template drew it, while the click handler
wrote the same sentence into a second element underneath. Anyone retrying a failed
verification saw the identical line twice, in two different colours.
One element now owns the outcome, and the handler replaces its text. Also colours the
in-flight "Checking DNS…" as muted rather than leaving it red.
Found by looking at a screenshot. Parsing, VM rendering and mutation testing all passed
over it — none of them draws anything.
18/18 browser checks, 1598 unit tests.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
c91b96ab91 |
SSO: refuse delegated proof names, release lapsed and deleted claims
Third review pass. It confirmed the crash wrapper holds (~13,000 hostile requests,
no fourth crash), the SSRF rewrite holds (77 vectors, every CIDR boundary proven),
the rate-limiter rewrite closed the login brute-force bypass, and /sso/claim rejects
every wrong token kind. It also found that two things I built yesterday did not do
what they claimed.
THE 8-HOUR LIMIT DID NOT BOUND SQUATTING
Pressing Verify on an expired claim REISSUED it in place, renewing the clock — so one
request per window held a domain forever, through the endpoint meant to enforce the
limit. Worse, a renewal was not a new claim, so the operator was notified exactly once,
on day zero: a tenant could sit on a company's domain for a year off a single stale
alert. A lapsed claim is now RELEASED. Re-adding it is an ordinary new claim: new
token, and the operator is told again. Squatting is not impossible; it is loud.
A DELEGATED PROOF NAME COULD FORGE A DOMAIN
A TXT lookup follows CNAMEs, and RFC 4592 means a wildcard `*.victim.com` synthesizes
`_screentinker-verify.victim.com` too — so a wildcard CNAME let whoever controls its
target prove a domain they do not own, turning an ordinary subdomain takeover into
every `@victim.com` login. A reviewer did this against a real authoritative zone. The
proof name is now refused if it is a CNAME, which is stricter than ACME's dns-01, and
the comment that claimed wildcards "cannot be mistaken for a proof" — true only for
wildcard TXT — has been corrected.
MY VERIFY BUTTON REPORTED FAILURE ON SUCCESS
`await load()` — the loader is `loadSso()`. The ReferenceError went into a bare catch,
so a correct DNS proof showed "Could not verify that domain" and left the card stale.
On the expired branch the admin kept publishing a token the server had already rotated.
ALSO FIXED
- deleting a provider stranded its verified domains (no FK, UNIQUE, never expires) so
the domain was blocked for EVERY org forever with no in-product recovery, and its
users could neither sign in nor reset. Delete now releases the domains and returns
the accounts to local, in one transaction; a cascade FK backstops it.
- isOrphanedFederated read absence-of-config as proof-of-deletion, so unsetting
GOOGLE_CLIENT_ID made every Google account password-resettable instance-wide, and
irreversibly. Restricted to org-provider slugs.
- `email_domains: null` (not undefined) took the destructive branch and deleted every
DNS proof an organization had.
- unbounded domain lists: 400 domains sent 401 emails; now capped at 50, one digest
per save, and /api/organizations is rate-limited at all for the first time.
- login and register responses carried password_reset_hash and email_verify_hash —
live account-takeover credentials handed to the browser. One sanitiser now.
- trailing-dot hostname (`https://localhost./`) slipped the SSRF guard.
- asyncRoute's own catch could throw and kill the process it exists to protect.
- a legacy DB whose typed domains were never verified now says so LOUDLY at boot
instead of silently locking every federated user out.
TESTS
Two of the previous round's tests passed against the code they were named after: one
asserted UNIQUE against the test harness's own CREATE TABLE rather than the shipped
schema, the other used two different domains so no ordering was exercised. Both
replaced and confirmed load-bearing. Seven mutations now turn the suite red, including
removing the CNAME refusal, the verified_at filter, and the expiry itself.
1598 tests pass. Delete-release, lapse-release and the leak fix verified against a
running server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
9155370ae8 |
SSO: TXT only for domain proof, drop the CNAME form
The CNAME alternative pointed at `<token>.verify.screentinker.com`. Making that work means operating a wildcard DNS zone that answers for every token ever issued — which this project does not have, so half the published instructions described a check that could never pass. Documenting a verification path that cannot succeed is worse than offering one form. TXT needs nothing outside the customer's own zone, and the dedicated `_`-prefixed name keeps it away from the apex where SPF and DMARC live. A wildcard `*.example.com` cannot be mistaken for a proof either way: it answers with its own value, never the token, so it lands in "exists but does not match". Also simplifies check() — one lookup, no Promise.allSettled, and NXDOMAIN is reported as "not published yet" rather than as an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
|
ScreenTinker
|
d4b8d7dad4 |
SSO: prove domain ownership by DNS, and fix what the second review found
A second review pass, run against the previous commit, found four blockers — two of
them introduced by the fixes in that commit. It also confirmed the original account
takeover is closed: a hostile IdP with real TLS, discovery, JWKS and RS256 driving the
real routers now stops at domain_not_allowed, and all 16 bypass variants are refused.
DOMAIN OWNERSHIP (the root cause, not the symptom)
A claimed domain used to mean "nobody else claimed it". It now means the organization
published a record in that domain's own DNS — TXT or CNAME, at a dedicated
_screentinker-verify name rather than the apex, where an edit would sit beside SPF.
- an unverified domain routes NOBODY and cannot be asserted; it reserves the name
- an unverified claim LAPSES after 8 hours, so a domain cannot be held against its
real owner, and lapsing rotates the token so a record left over from an abandoned
attempt cannot satisfy a later claim
- a verified domain never expires — re-proving on a timer would log a customer out
over a DNS edit made months later
- routing and confinement read the VERIFIED set only, never the typed column
- configuring SSO now requires a verified email address
- platform admins are emailed when a domain is claimed; nothing is ever sent to the
claimed domain, which would let any tenant make this product email third parties
Instance-wide providers are exempt from all of it: they are the operator's own
configuration and keep the trust they have always had.
BLOCKERS FROM THE REVIEW
- two unauthenticated remote crashes, both one request, both "async handler throws
before its try": `Cookie: st_oidc_tx=%` (unguarded decodeURIComponent) and the
fail-closed secret added last commit, which turned a JWT_SECRET rotation into a
permanent crash loop. Fixed the CLASS with asyncRoute() rather than the instances.
- the SSRF guard was bypassable via IPv4-mapped IPv6 ([::ffff:127.0.0.1]) and also
refused every host beginning "fc"/"fd" (fcm.googleapis.com). Addresses are now
parsed and compared by RANGE. 42 cases verified.
- the takeover fix had NO test — the test named after it asserted two struct fields
and passed with the guard deleted. The decision is now a pure function and four
mutations were confirmed to turn the suite red.
- the PUT path never received the TOCTOU fix, so two orgs could end up holding one
domain and forEmail handed routing to the attacker's older row.
ALSO
- linking compared slugs, so an org could never rotate its own IdP, and fell open on
an empty auth_provider. It now asks which ORGANIZATION owns the slug.
- an account stranded by a deleted provider can be reclaimed by password reset —
proof of the mailbox, which is stronger than the IdP assertion that created it.
- /sso/claim accepted a pre-TOTP mfa_pending token and returned the full user row;
it now takes a purpose-built 120s claim token with a pinned algorithm and typ.
- the rate limiter keyed on a caller-controlled path, so a trailing slash bought a
fresh bucket — a real login brute-force bypass.
- domain_not_allowed and account_exists_other_provider rendered as "please try
again", advice that can never work.
- malformed asserted addresses are refused rather than trimmed into shape.
- dead config (microsoftTenantId defaulted to 'common', which the provider code now
refuses) and the orphaned google-auth-library dependency removed.
1591 tests pass. Domain lifecycle verified end to end against a running server.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
d26aaebef6 |
SSO: fix an account takeover, a remote crash, and login CSRF found in review
Five reviewers went at the two SSO commits. Three of them independently
demonstrated a full account takeover, and it was the same defect each time.
TAKEOVER. An org admin supplies the issuer and client_id, so they control that
identity provider completely and can mint an id_token asserting ANY email with
email_verified:true — including a platform_admin's. Every cryptographic check
passed honestly, because the attacker IS the issuer. upsertFederatedUser then
re-pointed the existing account at whichever provider spoke last, because the
only guard was `password_hash IS NULL` — and every SSO-created account has a
null password. Sessions were issued as the victim, and the victim's own login
then failed forever with subject_mismatch.
The rule came from the old Google handler, where it was safe: only the operator
could add a provider. Making providers customer-configurable turned it into a
takeover primitive and the assumption was not re-examined. Now an org provider
may only assert emails inside the domains it registered, and may never adopt an
account another provider established.
REMOTE CRASH, unauthenticated. The state comparison guarded on UTF-16 character
length while Buffer.from produces UTF-8 bytes, so a state of 43 characters
containing one multi-byte character reached timingSafeEqual with mismatched
buffers and threw — inside an async handler, which Express does not catch, which
server.js turns into process.exit. One request per restart killed any instance
with SSO enabled. Compared as bytes now, and /api/auth/oidc gained a rate limit.
LOGIN CSRF. The callback returned the session token in the URL fragment, so a
crafted link installed an ATTACKER'S token and silently signed the victim into
their account. The token now goes in a one-shot httpOnly cookie exchanged at
POST /sso/claim, which a link cannot forge.
FRONTEND, dead on arrival twice over. login.js used `await` in a non-async
function — a SyntaxError that takes the WHOLE app down, since app.js imports it
statically and there is no bundler. And `esc` was never imported, so the org-SSO
button could never render; the ReferenceError was swallowed by the catch written
for network failures. Both slipped through because `node --check` parses these
files as CommonJS and exits 0 on a broken module. The correct check is
`node --input-type=module --check`, and all four frontend files now pass it.
PUBLIC EMAIL DOMAINS cannot be claimed. A tenant had claimed gmail.com in
review, after which every Gmail user typing their address was offered "sign in
with your organization" pointing at that tenant's infrastructure — phishing from
this product's own login page. server/lib/public-email-domains.js.
MICROSOFT multi-tenant is refused rather than silently broken. `common` metadata
advertises the literal template {tenantid}, so the issuer never matches and
every login already failed; and loosening that check is nOAuth. A tenant GUID is
now required, with a loud warning at boot.
SSRF: https only, loopback/RFC1918/link-local refused, redirects not followed,
and the test endpoint no longer echoes upstream status for a caller-supplied
jwks_uri (it was a readable internal port scanner).
Also: an omitted email_verified was accepted (the comment already said it should
not be); the domain-uniqueness check raced an 8s network call before its insert
and is now inside the transaction; same-org duplicate domains were allowed and
made routing depend on table-scan order; routing is now ordered; a client secret
that cannot be decrypted fails closed instead of silently downgrading to a public
client; SSO audit rows were writing the org id into the deviceId column; and
/sso/start was capped at 10/min per IP, which would 429 the 11th employee behind
a corporate NAT.
Adds per-provider editing in the org admin UI (replace-only secrets — never
returned, blank means keep, explicit clear) and a Test button that checks
discovery, endpoints and signing keys while stating plainly that it cannot
verify the client ID, the secret, or the redirect URI registration.
⚠️ STILL MISSING: domain-ownership verification. A claimed domain means "nobody
else had claimed it", not "they own it". DNS TXT proof is the remaining control.
1582 tests pass. New regression tests cover the takeover confinement, ordering,
fail-closed secrets, the Microsoft refusal and the public-domain blocklist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
 Christopher Cookman
|
1e329b0b80
|
Remove focus timeout for input element
Remove input focus timeout on overlay load. |
||
|
ScreenTinker
|
e97228a502 |
SSO: per-organization providers, configured by the customer
Instance-wide providers belong to whoever runs the server. These belong to a CUSTOMER: an organization points ScreenTinker at its own identity provider from Settings → Single sign-on, with no environment variable and no restart. The login flow is unchanged. An org provider is resolved through the same oidc-providers.get(slug) the env ones go through, so there is one authorization request builder, one token exchange and one verifier — not a second, less tested path for tenants. That seam is why Phase 1 put provider lookup behind a single function. ⚠️ An org provider is NEVER published. It is not in /api/auth/providers, because listing a customer's IdP would both offer it to people it does not belong to and leak the customer list from the login page. It surfaces only when someone types an address at one of that organization's domains; otherwise the instance-wide buttons are what you get. The discovery endpoint answers with a BOOLEAN and nothing else — no slug, no display name. Returning "yes, Acme Corp SSO" would turn a guessed domain into confirmation that Acme buys this product, and the slug would hand out a working entry point to their tenant. POST /sso/start repeats the lookup server-side and redirects, so the browser never learns which provider it is being sent to until the provider says so, and the address travels in a body rather than in a URL that lands in history, proxy logs and a Referer. Both endpoints rate limited to 10/min. Other properties, each with a test: - slugs are RANDOM, not chosen, so two customers cannot collide on or guess each other's URL - a domain may be claimed by ONE organization; a second claim is refused, so a tenant cannot capture another company's logins - the issuer is verified by live discovery BEFORE the row is written, so a typo is caught at configuration rather than by a user staring at a failed login - client secrets are optional (PKCE), stored AES-256-GCM via lib/secretbox, never returned; an absent secret on update leaves the stored one alone, which is how a settings form that cannot show it avoids blanking it - cross-org access answers 404, not 403, so an outsider cannot confirm that an organization id exists - signing in through an org provider grants membership of that organization, but never changes an existing member's role Verified live end to end: creation against a real issuer, domain normalisation (`@Acme.CO.UK` → `acme.co.uk`), boolean-only discovery, a rejected domain squat, a rejected bad issuer, and 404 for a foreign organization. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
|
Christopher Cookman
|
a88687a11b
|
Prevent input focus from scrolling
Remove focus from input to prevent scrolling behavior. |
||
|
ScreenTinker
|
252854d31e |
SSO: one OIDC flow for every provider, and verify the token properly
The OAuth support that was here could not work and would not have been safe if it had. It could not work: the login page called google.accounts.oauth2 and new msal.PublicClientApplication, and NEITHER SDK WAS EVER LOADED by any page in this app — no script tag, no dynamic import, nothing. Both buttons threw ReferenceError on click. Even had they loaded, the CSP allows scripts only from 'self' and cloudflareinsights, and frames only from self and YouTube, so the libraries and their popups were blocked too. It would not have been safe: both endpoints authenticated with an ACCESS token and neither checked who it was issued for. POST /auth/google fell back to tokeninfo?access_token= and read the email out of the reply; POST /auth/microsoft handed the bearer token to Graph /me and trusted that. Graph and tokeninfo will both describe the user behind a token minted for SOMEBODY ELSE'S application, so any site a user signed into that requested `email` or `User.Read` could have replayed their token here and been issued a session as them. Both endpoints are deleted; nothing is lost, because nothing could reach them. Replaced by ONE generic flow — Authorization Code + PKCE (S256), run server-side, with the provider list resolved through a single function so per-organization SSO can extend it later without a second login path. Google and Microsoft become ordinary configured providers; Okta, Keycloak, Authentik, Auth0 and anything else that speaks OIDC now work with three env vars. Because the exchange happens server-side the browser never talks to the provider, so there is no SDK to load, no client id in the page, and no third-party origin needed in the CSP. Identity comes from an ID token that must survive: signature against the provider's JWKS (asymmetric algorithms only — alg:none and HMAC are refused outright, the latter because the only key we hold is public), `iss` exactly as discovered, `aud` and `azp` matching our client, `exp`, and a `nonce` this server minted for that login. State is compared in constant time against a value in an httpOnly SameSite=Lax cookie, so the callback is CSRF-protected and survives a restart mid-login. Account rules are the ones already in place: a verified email is required, an SSO login never takes over an account that has a password, and a changed `sub` for a known address is refused rather than handing the account to a recycled mailbox. 18 new tests, every one describing something the old code would have accepted: cross-audience tokens, azp mismatch, replayed nonces, alg:none, HMAC forgery, wrong signing key, expired tokens, a discovery document lying about its issuer, and a registry that never leaks a client id or secret to the browser. Verified end to end against Google's real discovery document: the redirect carries response_type=code, PKCE S256, state and nonce, and every callback guard rejects as intended (no cookie, wrong state, no code, provider refusal, unknown provider). ⚠️ TOTP is still not prompted on an SSO login, matching the documented behaviour of the previous SSO and API-token paths. That is a product decision and is left unchanged here rather than altered silently. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A |
||
copilot-swe-agent[bot]
|
1e278ea373
|
Fix banner persistence across view switches and modal scroll on open
Co-authored-by: ChrisChrome <28414320+ChrisChrome@users.noreply.github.com> |
||
|
copilot-swe-agent[bot]
|
4fe2552971
|
Fix banners: prepend inside #app instead of inserting before it as sibling
Co-authored-by: ChrisChrome <28414320+ChrisChrome@users.noreply.github.com> |
||
|
copilot-swe-agent[bot]
|
9f83832f7d
|
Fix banners overlapping sidebar by adding margin-left matching sidebar width
Co-authored-by: ChrisChrome <28414320+ChrisChrome@users.noreply.github.com> |
||
|
copilot-swe-agent[bot]
|
f725186905
|
Add org-level widget sandbox isolation toggle with warnings
Co-authored-by: ChrisChrome <28414320+ChrisChrome@users.noreply.github.com> |
||
|
ScreenTinker
|
28885d1a13 |
Merge branch 'feat/brightsign-ip-from-js'
# Conflicts: # server/test/device-controls-hidden.test.js |
||
|
ScreenTinker
|
08b3d7d404 |
BrightSign: report IPv6, the attached display and the active video mode
Follow-on from the Node-stdlib work, guided by BrightSign's own dev-cookbook
rather than by guessing at module names.
IPv6 costs nothing extra — it comes from the same os.networkInterfaces() call
the v4 address does. The column, the API field and the dashboard card have all
existed since 1.9.29 and no player has ever filled them; the card is written to
appear ONLY when set, precisely so the overwhelmingly v4 fleet does not pay
screen space for an empty row. fe80:: is skipped for the same reason 169.254 is
— a link-local address is scoped to one interface and cannot be dialled from a
laptop across the office. A ULA is kept, because that one is reachable.
The attached display and video mode are new columns, and they answer the first
question anyone asks about a dark sign: which panel is that, and is the player
outputting at all. screen_width/height could not answer it — they are what the
PAGE believes it has, i.e. the widget's own geometry. Our XT245 drives a CX101
at 1920x1200@60 while the page reports its own canvas.
Per telemetry row rather than on `devices`, because a display can be swapped,
unplugged or renegotiated without the player re-registering.
MULTI-OUTPUT: the output is chosen by screen number, not hard-coded. A
dual-output player registers ONE DEVICE ROW PER OUTPUT (?screen=N →
output_index), so each row must report its own panel — otherwise a box driving
a lobby TV and a menu board shows the lobby TV twice. Both naming forms are
tried: probed on hardware, "hdmi" and "HDMI-1" both resolve to output 1, while
a second output that does not exist fails cleanly ("hdmi2" throws from the
constructor, "HDMI-2" rejects), so a single-output player reports nothing
rather than inventing a screen. That case has its own test.
Dashboard: two cards, shown only when the player reports them, like every other
card in that block.
Verified end to end on the real XT245 (FW 9.1.93.2) — attached_display=CX101,
video_mode=1920x1200@60, alongside local_ip 192.168.1.46, 119616 MB disk,
3656 MB RAM and live CPU.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
def30e6d39 |
BrightSign: report the LAN address, real disk, memory and load
Every one of these fields existed in the schema, the API and the dashboard,
and every one was NULL or misleading on a BrightSign. The XT245 had 6000
consecutive telemetry rows with local_ip NULL while sitting at a perfectly
reachable 192.168.1.46, and reported "1026 MB" of storage for a 119 GB NVMe.
The host half (autorun.brs) does collect an address, but nothing the host
sends was arriving at all — proven by the storage figure, which was the
browser's cache quota rather than any disk. So the page has to read this
itself, which is also the half that can be delivered: st-bridge.js is served
per page load, while autorun.brs needs a release bump to reach a player.
It is Node's standard library, not a @brightsign module. The widget is created
with nodejs_enabled, so os and fs are simply there — this is what BrightSign's
own dev-cookbook does in html5-app-template (both the .ts and .js variants).
Looking for a platform module is the trap, and it cost most of a day:
@brightsign/networkconfiguration EXISTS but exposes only callback,
getNeighborInformation and enableLeds — no config reader. hostconfiguration
has getConfig()/applyConfig() but returns host settings (forwardingEnabled,
hostName, loginPassword, nameServers) with no address in them. Both enumerated
on the live player, because the JavaScript API doc pages 404 and BrightSign's
own roNetworkConfiguration page links to one of the dead URLs.
getCurrentConfig() is BrightScript-only.
local_ip os.networkInterfaces(), skipping internal and 169.254
ram_total/free os.totalmem() / os.freemem()
cpu_usage 1-min load average / core count, as a clamped percentage
uptime_seconds os.uptime() — the MACHINE, overriding the page's own
performance.now(), so a widget rebuilt by the watchdog no
longer hides weeks of real uptime
storage_* fs.statfsSync over the mounts under /storage, largest wins
(ours boots from NVMe with a dead card slot; others from SD)
Dashboard: the RAM and CPU cards were gated on "is this Android?", which was
right when Android was the only family that could measure them. They now
render for any player that reports the value, so a BrightSign gets them and
Android is untouched — including keeping its "--" cards when no reading has
arrived, since an empty card is a known state and a missing one reads as
"cannot". The BrightSign storage card loses its "player storage" caveat,
because the number is now the disk it always claimed to be.
Verified on the real XT245 (FW 9.1.93.2): 116.8 GB free of 116.8 GB, 2.68 GB
of 3.57 GB RAM, 3% CPU, uptime tracking the machine, local_ip 192.168.1.46 —
matching the address found independently by MAC-vendor scan, and a disk figure
matching the kernel's own block count.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
3333d5e968 |
Stop Android panels losing controls when they update
A declared capability set REPLACES the per-platform baseline rather than
merging with it, so anything the baseline grants and the player omits is a
control the operator loses by updating. Three were being lost.
- display.brightness: the per-window dim (setWindowBrightness) is Tier 0 —
no permission, no owner, no WRITE_SETTINGS — and MainActivity applies it
unconditionally. It was simply never declared.
- remote.screenshot / remote.stream: gated on the accessibility service,
while captureScreen() falls through to ScreenshotCapture.captureView,
a plain view draw with no permission check. A Tier-0 panel lost live view
and screenshots by updating, and a GRANTED MediaProjection never became a
capability either — consent given, capture working, server still refusing,
because nothing re-declared.
- system.device_owner: no player declared it, so the server accepted
system.kiosk as a stand-in for every Tier-2 command. Declaring the
canonical name makes refusals say what they mean; the stand-in can retire
one release after this reaches displays.
display.power stays conditional on purpose: screen_on works anywhere via a
wake lock but screen_off needs owner/admin/accessibility, and a control that
sleeps a panel it cannot wake is worse than no control. It is the sole entry
in the DELIBERATE set in player-parity-baselines.test.js.
Also fixes the capture-bootstrap gate in device-detail.js. It hung off
can('remote.screenshot'), which hid the button from exactly the panels that
need it. The gate is now Android-and-nothing-else, NOT "Android that lacks
capture": /api/devices/:id ships capabilitiesFor(), which flattens declared
and baseline into one array, and the android baseline contains
remote.screenshot — so a "lacks capture" test hides the button from all ~440
undeclared panels in the field. isAndroidDevice() mirrors platformFamily()
with all four signals in order; an Android-test-only helper classified every
Tizen TV as Android, since Tizen registers android_version 'Tizen 6.5'.
Tests: the suite could not see any of this. Mutation testing showed deleting
either capability line, or reverting isAndroidDevice to its buggy form, left
all tests green. Added an update-invariant test (declared set vs baseline,
with an argued exception list), a test that executes the shipped helper
rather than the harness stub, and a legacy-panel test using the shape the API
actually returns instead of one it never produces. All four mutations now
fail.
Verified on a real Android 16 device across all three tiers: Tier 0 captures
live video (no accessibility, no MediaProjection, no owner), Tier 1 gains
display.power via accessibility, Tier 2 declares system.device_owner and every
Tier-2 command delivers. An in-place upgrade from the pre-change build lost
nothing and gained exactly these three.
Baselines deliberately NOT moved — a baseline entry moves in the release
AFTER the one carrying the player fix, once it has reached displays.
Parity gaps 3 and 4 were implemented, audited and reverted; docs/player-parity.md
records why so the next attempt starts from the traps. Gap 3 (wiring "Force
update") meets an unbounded synchronous download against a 120s watchdog and a
3-attempt counter with no version binding, so three presses refuse a panel every
future version. Gap 4 (deferring to BS.capabilities()) removes working
screenshot/stream from diskless BrightSigns that capture to RAM, over-declares
transitions, and rides a probe timeout that discards a late answer permanently.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Bvjey4FNam49MN7ybjcq6A
|
||
|
ScreenTinker
|
047f95f40c |
Freeze and copy the live debug log, and unstick the control row
Three small things off the device page. The control row had margin-top but no margin-bottom, so the buttons sat flush on top of the STATUS card and the destructive ones read as part of the status panel. Freeze is the one with a decision in it: it holds the VIEW still and keeps buffering underneath rather than pausing the stream. The moment you freeze a log to read something is the exact moment the lines that explain it are still arriving, so dropping them would throw away the part you were about to want. Resume replays them in order. The held buffer is capped at the same 500 as the panel, and the status text says how many are waiting -- otherwise a frozen panel is indistinguishable from a device that went quiet, and silence reads as a symptom. Overflow says so too. Copy takes what is on screen (not the held lines -- the paste must agree with the panel) and stamps it with the device and an ISO timestamp, because a pasted log with no device in it is a log nobody can act on. It falls back to execCommand when navigator.clipboard is absent, which is every self-hosted dashboard on plain http: that is not a secure context, and the other copy buttons in this app quietly do nothing there. Clear earns its place next to Copy: without it you always copy 500 lines of history instead of the capture you just made. The hint promised the stream "turns off on its own when the device reconnects", which was never true and is not what happens now -- it turns off when you leave the screen, and on the device after 30 minutes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS |
||
|
ScreenTinker
|
c594a1a67a |
Make the live debug log work on the web player, and so on BrightSign
The dashboard's per-device "Debug logging" checkbox has always sent a `set_debug` command. The Android player honours it — DebugLog.* mirrors its tagged lines over the device socket while the box is ticked. The web player never implemented the command at all, so the panel opened, revealed itself, and streamed nothing but the three unconditional reporters (sync, pip, zone). A display could be failing loudly in its own console and look mute from the dashboard. In a browser that is a nuisance — press F12. On BrightSign it is the whole diagnostic surface: no console, no adb, no logcat, a panel on a wall. Rather than hand-instrument eighty-seven call sites to match Android's tag by tag, this streams the ring buffer the error trap at the top of <head> has always filled: every console.log/warn/error, every uncaught error with file:line and stack, every unhandled rejection, every failed resource load. Turning the stream on also REPLAYS that backlog, so the operator sees the failure that happened before they opened the screen — the case they actually came to investigate, and one no log tail gives them. Replayed lines carry their real age, because the dashboard stamps on arrival and 200 lines would otherwise all claim to have happened this second. The bracket prefixes the player already uses ([wall], [bs], [group-sync]) become the tag column, so the panel reads the same shape as Android's, and the panel now colours by level — all four rendered identically before, so the one line explaining the fault sat in a wall of grey. Bounded three ways, because this sink is fed by console.*: - 40 lines/sec, over which lines are COUNTED and reported, not queued - auto-off after 30 min, for the checkbox nobody unticks - the dashboard also switches it off when the operator leaves the screen The reentrancy guard in pushLog is not theoretical: the sink runs inside the console wrapper, so a subscriber that logs anything would recurse until the stack gave out and the player would die of its own diagnostics. BrightSign host lines stand their direct emit down while the stream is on (the console path already carries them) but still go out unconditionally when it is off — the boot report is the one diagnostic nobody can ask for in advance, because it is over before the operator has a device to open. Verified on the XT245 on alpha: 34 lines across 7 tags, backlog replayed with real ages, levels intact, platform line reporting BOS 9.1.93.2 / XT245 / 1920x1200. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS |
||
|
ScreenTinker
|
ab44a2efc6 |
Put the device controls where the operator is looking
Reboot, screen off/on, launch player, force update and shutdown sat at the bottom of the Info tab, below the info grid, the uptime timeline, the incident list, the reboot schedule and the debug log panel. They are the actions someone opens a device page to take, and reaching them meant scrolling past everything that merely describes the display — worst on a phone, which is where an operator standing in front of a dark screen actually is. Moved to the top of the tab, directly under the diagnostics panel and above the info grid. Still one wrapping row, so a narrow screen reflows instead of clipping, and each button still renders only where the display can honour it — the capability gating is untouched, so a panel that cannot reboot still shows no reboot button. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS |
||
|
screentinker
|
a6a137daf2
|
Merge pull request #246 from screentinker/feat/ipv6-and-panel-scaling
Show a panel's IPv6, and size the pairing code to the screen it is on |
||
|
ScreenTinker
|
9face2fdd4 |
Show a panel's IPv6, and size the pairing code to the screen it is on
Two field-reported gaps, unrelated except that both are about being able to read something off a screen. A PANEL'S IPv6 WAS NEVER COLLECTED, LET ALONE SHOWN. DeviceInfo.getLocalIp() filters to Inet4Address, so a v6-only panel reported no address at all and the dashboard rendered a dash for a screen that was perfectly reachable. It now reports both stacks in their own fields: a dual-stack panel genuinely has two addresses and either may be the one you need, so collapsing them into one column would make it mean "whichever interface enumerated first". Link-local (fe80::/10) is deliberately excluded. Every interface has one, they tend to enumerate first, and none can be dialled without also knowing the zone index — so admitting them would fill the field with a string nobody can paste anywhere and hide the address that works. Any %iface suffix is trimmed for the same reason. The 45-char cap the writer already applied is exactly the longest legitimate IPv6 text form, so it needed no change. The dashboard card renders only when a panel actually has a v6 address, rather than showing an empty row to the overwhelmingly v4 fleet. THE PAIRING CODE DID NOT SCALE, WHICH IS WORST WHERE IT MATTERS MOST. Every size on the pre-playback screens was a hard-coded pixel value. A CSS pixel covers a quarter of the screen area on a 4K panel that it does on 1080p, and a sixteenth on 8K — so the 72px code that fills a 1080p screen is a smudge on the 4K wall it was installed on, which is where signage actually goes. What has to stay constant is ANGULAR size, so the root font size is now viewport-proportional and everything on those screens is a rem against it. The code holds 6.67% of screen height at every resolution: 72px at 1080p — bit for bit what it renders today, so nothing changes for the existing fleet — 144px at 4K, 288px at 8K. Verified in a browser rather than by arithmetic: at a 1409px viewport the root computes to 13.0473px, which is 0.926vmin to four decimals. vmin, not vw, because portrait-mounted panels are common here and vw would render a 1080x1920 screen at half size. Clamped at both ends so the dashboard's preview iframe stays legible instead of microscopic and an ultrawide does not get silly. Applied to the web player (which BrightSign also runs) and to Tizen, where a 1920x1080 logical viewport makes it arithmetically identical to the values it replaces — the point being the panels where it is not. A test asserts the scaling cannot reach playback content: the whole safety argument is that only the chrome uses rem, and a stage or zone rule adopting it would start resizing CONTENT, which is a worse bug than the one being fixed. Android is untouched — its pairing code already autosizes within a dp-scaled layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014skWYXJUWhF73EvNPgB2AS |
||
Rob K
|
0f2ec474f4 |
Surface the screenshot-request verdict as a toast
The server already acks dashboard:request-screenshot with
{ delivered, reason } (offline / unsupported via the capability
registry), but no dashboard sender passed a callback, so clicking
Screenshot on an offline device or an unsupporting player type showed
"Screenshot requested" and then silently did nothing.
requestScreenshot() now takes an optional callback using the same
.timeout(5000) pattern as sendCommand(); the device-detail Screenshot
button passes one and toasts the verdict (requested / unsupported /
offline / no response). The dashboard grid and the 5s Now Playing poll
keep firing-and-forgetting - no behavior change there.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0131RYmVh8ePEhparD3mXBhU
|
||
|
ScreenTinker
|
3e37d33b80 |
QA: close four ways a control or an asset lied about itself
Found by driving the real server and a real browser, not by reading. Each fix has a
test that fails without it.
1. A missing upload answered 200 with the DASHBOARD. express.static falls through on a
miss and the SPA catch-all caught it, so GET /uploads/content/<gone>.mp4 returned
15KB of index.html as text/html — under the `immutable, max-age=30d` header the mount
sets before it knows the file exists. Every player downloader treats 200 as success,
so a panel stores the HTML page AS the video and caches it for a month, rendering a
black frame with nothing in any log. Reachable exactly when it hurts: a content
replace writes a new random filename and unlinks the old one. The mount now
terminates a miss with a 404 and drops the cache header.
2. Four dashboard->device socket handlers had no capability gate. dashboard:device-command
has always refused a command the panel cannot honour, and the comment above it is right
about why ("hiding the button is not enforcement — this socket is reachable directly").
Every word applied to the four handlers immediately above it, which had none: a display
declaring [] still received screenshot-request, remote-touch, remote-key and
remote-start. Measured, not inferred. They now refuse on remote.screenshot /
remote.input / remote.stream and name the capability in the ack; remote-stop stays
ungated for the same reason set_debug does. The undeclared fleet is unaffected — an
absent declaration still resolves to its platform baseline and keeps everything.
The wall panel list (#235) made this visible: it offered a Screenshot button for every
panel, including a BrightSign, which has no screenshot capability at all, and popped a
toast promising an image that was never coming. GET /api/devices now ships the RESOLVED
capability array rather than the raw column ('[]' as a STRING, which Array.isArray reads
as "pre-capability server, show everything" — wrong in the one case that matters), so
the wall list and the fleet cards can hide what a panel cannot do. The remote pad's
Scrn Off / Scrn On were gated on remote.input while the Info tab gated the same two
commands on display.power; both now agree.
3. A register with no `platform` ERASED the stored one. captureIdentity coerces a missing
field to the literal 'unknown' and persistIdentity wrote it straight over. That column
is load-bearing: platformFamily() reads it, so one reconnect from an older build turned
a Tizen panel into a browser tab and handed it a volume slider the .wgt has no handler
for — the exact control BASELINE.tizen exists to hide — while a BrightSign lost screen
power and reboot and gained screenshots it cannot take. platform and client_type are
now preserved (physical facts); client_version and contract_version still decay, because
there "we no longer know" is the truthful answer. client_type 'wgt' is also read as a
second signal for a Tizen TV.
4. PUT /api/content/:id/replace carried its own shorter copy of the ingest logic. Replacing
a video left duration_sec at the OLD clip's length and nulled width/height, so #237's
brand-new "default an item to the clip's own length" then handed out the wrong number
for every later add — 32s scheduled for a 5s video is 27s of frozen frame. Replacing an
image measured it with raw sharp metadata and thumbnailed without .rotate(),
re-introducing the EXIF-orientation bug #172 had just fixed at ingest. Both paths now
share lib/content-ingest.deriveMediaMetadata.
Verified working and NOT changed: all six item-duration insert paths (a 31.7s clip stores
32 everywhere, an explicit value always wins, and no path can store a 0); the content
revision bump + filepath refresh reaching a real device socket; a landscape wall producing
byte-identical geometry to the pre-#236 expression; a portrait wall reaching the player as
side-by-side halves; cross-workspace isolation across 29 probes.
Full suite green (1319).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|
||
|
ScreenTinker
|
2237edab12 | Merge #236/#235: portrait video walls, and a wall status view | ||
|
ScreenTinker
|
4d86a75196 |
Merge #239: let the playlist preview skip to any item
# Conflicts: # frontend/js/views/playlists.js |
||
|
ScreenTinker
|
97f53a5b72 | Merge #238: preview a rotated display the way the wall shows it | ||
Claude
|
e4c25c39df |
Describe a portrait video wall as portrait, and stop a wall hiding its screens
#236: the wall canvas was secretly framebuffer space rather than the wall as the audience sees it. Invisible while every panel is the normal way up, and actively misleading the moment one isn't — two portrait-mounted panels standing side by side had to be STACKED VERTICALLY in the editor, with a pre-rotated copy of every video, before the output came out right. It worked, but only after trial and error, and it meant a portrait wall could never reuse content as-is. Each panel now carries a mounting rotation (0/90/180/270 clockwise, the same convention as the per-device orientation setting), the canvas means the physical wall, and the player works out the mapping. The geometry lives in one place, server/lib/wall-geometry.js, because four players have to agree on it to the pixel across a seam. Existing walls need no migration and do not move. Every wall in the field is rotation 0, and that case takes the original expression verbatim on all three players rather than the algebraically-equal centre-based one — the two differ in the last float bit, and a float's worth of disagreement between two panels is a hairline seam down a wall that was aligned yesterday. Pinned by the first test in wall-geometry.test.js and by wall-payload.test.js. While a display is in a wall its panel rotation replaces its own orientation: both describe the same physical fact, so honouring both turned the content twice. #235: a wall replaced its members' cards, so one dead panel of a four-panel wall was invisible from the dashboard, and inspecting a single screen meant pulling it out of the live wall and putting it back. The wall screen now lists its panels with live online state, a per-panel screenshot request, and a link to each device's page; the dashboard wall card carries per-member status chips that track socket updates. Tests: wall-geometry.test.js re-simulates the CSS box independently and asserts each panel's viewport maps onto exactly its own rect of wall space, for every rotation, plus a mixed wall and the Tizen player's hand-ported copy executed against the canonical rule. Full server suite green (1260). Not verified here: the Android and Tizen renders on real hardware. Kotlin compiles clean; the maths is shared/tested, the view plumbing is not. |
||
|
ScreenTinker
|
52ab04204a |
Preview a rotated display the way people see it, not the way its framebuffer is
#238: the dashboard preview of a 90/270 display was sideways while the panel on the wall was right — the split that makes a preview useless, because a designer checking portrait content can no longer tell a real fault from an artefact of the tool. A portrait panel is a landscape framebuffer that the player rotates content INSIDE (+90), hung turned the other way (-90); the two cancel and the viewer sees upright portrait. The dashboard modelled only the first half. It iframed the player into a box it had already given the finished 9/16 shape, so the player rotated a second time inside a box that was pretending to be the finished picture, and nothing anywhere stood in for the mount. Screenshots had the opposite half missing: they are the raw framebuffer, shown untouched, so every portrait screen looked wrong on the cards and in Now Playing too. So each surface now has a stage (the panel's face) and a frame (its framebuffer), with the frame turned by the INVERSE of the player's angle. Turning it the same way is the tempting mistake and the worst kind of wrong: 90+90 lands upside-down, which reads as nearly-right. The dimension swap is not cosmetic either — composing into the real framebuffer shape is what makes the player lay content out in the same portrait box the panel uses; hand it a portrait viewport instead and every zone and object-fit decision is computed for a canvas no panel has. The geometry is the players' own rule (server/lib/orientation-style.js), served to the dashboard rather than re-derived, since a second copy of a rotation rule is exactly how the two came to disagree. Covers the device preview modal, the playlist preview's portrait toggle (same fault), Now Playing and the device cards. The Remote canvas stays raw on purpose: taps are sent as fractions of it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
ScreenTinker
|
6471c503ab |
Default a video playlist item to the clip's own length (#237)
Adding a 32s video gave it the flat 10s default, so it was cut off mid-play unless the operator looked up the runtime and typed it — per item, every time. The content row already carries the probed duration; it now becomes the default. The rule lives in one place (lib/item-duration.js) because the operator sees one product, not six insert paths: playlist add, assign-to-display, group assign, agency portal, content-only schedule, and the public API all share it. Only the playlist route defaulted before, and it stored the raw probe (31.7) which the Android player's optInt read silently truncated back to 31. Explicit values always win. Content with no trustworthy duration (image, widget, YouTube, remote URL, failed probe) keeps the 10s default, and a duration that is 0/negative/NaN or absurd (> 12h, i.e. a broken probe) falls back rather than reaching a device — a 0 makes the players schedule a 0ms advance, which self-loops and black-screens the TV. Dashboard: the add-item picker shows a clip's length, the assign-to-display modal pre-fills the duration field from the selected clip (never overwriting a value the operator typed), and onboarding stops hardcoding 10 on the first assignment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
ScreenTinker
|
aa77332c0d |
Let the playlist preview skip, so reviewing item 8 does not cost seven durations
The preview shipped without the skip control #104 asked for, so checking a late item meant watching every item before it in real time — the thing operators do most when ordering a playlist with a client on the phone. The preview is already the real player in device-free mode (an iframe of /player?preview=1), so this drives that instance rather than growing a second playback implementation: the dashboard posts next/prev to the one contentWindow, the player steps its own currentIndex and re-renders through the same path a natural advance uses, and posts back index/total so the modal can say "3 of 7". Nothing here can reach a live screen. A real display is driven over its server socket and holds no window handle this page could address; the message listener is installed only by the preview boot path, previewNavigate refuses outside PREVIEW_MODE, and both ends pin the origin. Stepping is schedule-aware in the direction of travel — falling forward past a dayparted item would make "previous" walk forwards — and a multi-zone playlist reports itself as such, because all zones play at once and a counter there would be a lie. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
ScreenTinker
|
0082191f9b |
Show only the controls a display can actually honour
Every device control was offered to every display. A browser tab was shown "Reboot device", a Tizen TV was shown screen power, a player with no framebuffer read was shown a live view that stayed black. They all looked like working buttons and did nothing — the "reports success and changes nothing" shape that keeps costing people days. Players now declare what they can do at registration, because only the player knows at runtime: an Android panel gains real screenshots when accessibility is switched on and loses Tier-2 when device owner is revoked. The dashboard hides what is not supported rather than disabling it, and the Info tab lists the capability set so a missing control is explainable. The declaration is three-state and the middle state is load bearing: NULL means "has never told us anything" and falls back to a per-platform baseline, because several hundred displays in the field will not update before this deploys and blanking their controls would be a far worse bug. An empty array means "I genuinely can do nothing" and is honoured. Hiding a button is not enforcement, so unsupported commands are also refused server-side — the socket is reachable directly and a stale tab still renders the old controls. Group sends report skipped devices separately from sent ones; counting an unreachable member as "sent" is how an operator walks away believing the whole group rebooted. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
ScreenTinker
|
d205a49dfa |
The settings PIN can be rotated and set from the dashboard
It was generated once at pairing and never changed. On a fleet that makes it a
shared secret with no expiry: anyone who watches it typed once — an installer, a
contractor, someone filming a screen — keeps it for the life of the panel, and
the only way to take it back was to unpair and re-pair every affected display. A
customer asked whether it rotates, which was the right question.
POST /api/devices/:id/settings-pin takes { rotate: true } or { pin: "123456" },
and pushes the result to the panel over its socket immediately. The live push is
the part that matters: without it a new PIN would only take effect at the next
pairing, so an operator revoking a leaked PIN would believe access was closed
while the old one still opened the menu. The response reports whether the panel
actually took it, so an offline display is stated rather than assumed.
Validation is the security-relevant half and is pure and tested: six digits,
digits only, and a blocklist of the PINs people actually pick (repeats and
sequences) refused on explicit set and never produced by the generator. A PIN
that can be set to "0000" or left empty is a gate that is not there.
Generation uses crypto.randomInt rather than Math.random — this is a credential,
and a rotation requested BECAUSE a PIN leaked must not be predictable from
anything else. Leading zeros are padded, or roughly one PIN in ten would be five
digits and rejected by the on-device prompt.
Android applies it live via device:settings-pin instead of only at pairing. The
PIN is never written to a log on either side, and it stays out of device list
responses as before.
1084 pass; Android compiles.
Asked for by chris@chris-pc.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|
||
|
ScreenTinker
|
803f4ec26d |
Portrait templates, a canvas that matches the layout, and a playlist mockup
Three related pieces. Zones were already stored as percentages and layouts already carried their own width/height, so this is mostly design work rather than plumbing. SIX PORTRAIT TEMPLATES at 1080x1920. Deliberately not the landscape set turned sideways: "Three Column" at 33% each becomes three tall slivers, and a 15% ticker that reads well across 1080px is a 288px band on a 1920px-tall panel, so the portrait ticker is 12% and the PiP window is wider than tall (a 30x30 box is square on 16:9 and 324x576 in portrait). Seeded in schema.sql for fresh installs AND as a migration, because schema.sql never runs on an existing database — and upgraded instances are exactly the ones with portrait panels already deployed. THE EDITOR CANVAS followed a hardcoded padding-top:56.25% — the 16:9 ratio trick. Authoring a portrait layout meant dragging zones on a landscape canvas: the percentages landed correctly on the panel and looked wrong everywhere you designed them. It now derives from the layout's own height/width, clamped so a pathological row cannot produce an unusable editor. THE PLAYLIST PAGE now draws where content actually lands. A playlist has no intrinsic layout, so the server reuses #104's derivation from the items' own zone bindings and returns it. Previously an item could be tagged "Bottom Ticker" with nothing to say the ticker is a thin strip along the bottom — people assigned by zone name and found out by looking at a screen. Empty zones are dimmed, because an empty zone shows its background colour on a real panel and that is worth seeing before publishing rather than after. Verified against a copy of prod: 6 templates and 12 zones created, the 7 landscape templates untouched, no errors at boot, and a second boot changes nothing. Each stacked template's zone heights sum to exactly 100%. 1074 pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
ScreenTinker
|
4ed7954f84 |
Drop the user-agent fallback — it could never fire
isBrightSignDevice() fell back to device.user_agent to catch panels paired before this port existed, which registered as "Chrome 120" with a BrightSign user agent. `devices` has no user_agent column, so the field is always undefined on a row read from the database. The branch was unreachable in production and passed only in a test that fabricated the field — which is precisely how dead code survives review. Two agents flagged it independently while working on unrelated areas, and the schema confirms it: zero matches for user_agent in the devices table. Those pre-port panels are recognised the moment they re-register on a build carrying the host, which every one of them gets on its next update. Identifying them sooner would mean persisting the user agent, and a column added solely to track a population that disappears on its own is not worth carrying. The test now asserts the honest behaviour: a fabricated user_agent does NOT create a match, and a group containing such a panel reads as mixed until it re-registers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
ScreenTinker
|
16b3dd949c | Merge: BrightSign real telemetry and hardware identity | ||
|
ScreenTinker
|
5a7277523a |
Wire BrightSign native sync end to end, chosen per group
st-sync.js wrapped SyncManager but nothing drove it. The player now does: the
leader opens a new sync session on each advance, and every member — the leader
included — binds the video with attachVideo() on a NEW id only.
The leader binds from its own broadcast rather than at announce() time on
purpose. Starting when it announces would put it ahead of its followers by the
width of the network, which is the one desync nobody would think to look for
because the leader always looks correct.
Item selection stays clock-derived under both backends. Native sync replaces
only the seek/nudge drift correction, because setSyncParams has the element hold
its own alignment and correcting it ourselves would fight the platform — every
frame we moved is one it then has to undo. Keeping selection on the shared clock
is also what keeps images and widgets, which have no setSyncParams, advancing
with the videos instead of drifting off alone.
LEADER RULE: reuse the existing election (resolveGroupLeader) rather than adding
a column. It already resolves pinned-if-online, else first online member on the
shared playlist, else first by id — deterministic, stable, and already what the
group-sync payload reports. A second mechanism could only disagree with it.
Added on top: a group whose elected leader is OFFLINE falls back to our
protocol. Ours is leaderless and carries on; native sync has exactly one
broadcaster, so those members would sit waiting for an announcement that never
comes, with the dashboard showing a healthy group throughout.
device_groups.sync_backend ('auto'|'screentinker'|'brightsign') is the operator's
REQUEST; the answer comes from the existing pure resolveSyncBackend() so the
players, the dashboard and the stored setting cannot disagree. The resolved
backend, reason and downgraded flag ride in the group_sync payload and in the
group API, and the dashboard shows the refusal reason instead of a setting that
quietly isn't in force. An unrecognised value is rejected rather than stored,
because the resolver reads anything unknown as 'auto' — a typo would otherwise
return 200 and run a different protocol than the UI displayed.
FIXED WHILE HERE: the player re-entered group sync only when the group ID
changed, with a comment noting the clock protocol has no leader role. Native
sync has one, and neither a protocol switch nor leadership moving alters the
group id — so a player promoted to leader kept behaving as a follower, nobody
announced, and the group sat unsynchronised. The re-enter key now includes the
backend and the leader flag.
971 pass (+17).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|
||
|
ScreenTinker
|
46b2227dfd |
BrightSign: real telemetry and hardware identity, not a block of nulls
The web player sent battery/storage/RAM/CPU as nulls and 'Web Player' in the
wifi_ssid column, and the bs_model / bs_os_version / bs_serial / bs_screen fields
the player already reported at registration were consumed by nothing. A browser
tab genuinely has none of that. A BrightSign has some of it, and was reporting
none.
Telemetry comes from a CACHE the heartbeat reads synchronously. The beat builds
its payload every 15s without awaiting, but the one real sensor here —
deviceInfo.getTemperature() — returns a promise; awaiting inside the beat would
either block it or serialise a pending Promise into the payload, which is exactly
how device_id once became "[object Promise]". The cache starts EMPTY rather than
null-filled and is spread last, so off-platform nothing changes and a null here
can never clobber a value another player family legitimately supplied.
wifi_ssid was actively false on a PoE Ethernet appliance — an operator reading
that column was told an SSID that does not exist. It is null there now, and the
device view shows a real hardware block instead. Android's WiFi display is
untouched.
Hardware identity is a SEPARATE writer from applyDeviceInfo, deliberately. That
function is a blind full-row overwrite, and an empty device_info once nulled
seventeen columns every five minutes because {} is truthy. These fields arrive
only on a full register, so the same shape would wipe them on every lightweight
refresh in between; COALESCE makes "no news" mean "unchanged".
The OS build gets its own column rather than reusing android_version, which is
load-bearing as a TYPE discriminator: device-detail chooses between the Android
and browser layouts with android_version.startsWith('Web/'), so writing
"BrightSign OS 9.0.189" there would have rendered a BrightSign with battery and
WiFi cards — and applyDeviceInfo would have clobbered it on the next refresh.
Storage is labelled "Player Storage", not "Storage": on this family the number is
the widget's cache quota, not the device filesystem, and it lands in the same
column as Android's real disk figures.
Schema: device_telemetry.temperature_c REAL; devices.hardware_model,
hardware_serial, hardware_os_version, output_index. All nullable, all idempotent
in the existing migration array.
973 pass (+19). The temperature tests drive a real socket into a real server,
because changing the arity of the telemetry INSERT would break every player's
heartbeat, not just BrightSign's.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|
||
|
ScreenTinker
|
ad18914736 |
Label BrightSign players as BrightSign, not "Web Player"
A BrightSign runs the same web player, so client_type is 'player' and the device
detail view fell through to a hardcoded "Web Player" — indistinguishable from a
browser tab on someone's desk, for a dedicated signage appliance.
Keyed on the platform the player now reports ('brightsign', from the
?platform=brightsign the host puts on the URL), with a user-agent fallback for
panels paired before that existed — those registered as "Chrome 120" with a
BrightSign user agent.
Only en carries the new string; other locales fall back to en, which reads
correctly since the label is a brand name.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|
||
|
Claude
|
a310d7d5b6 |
Stop the onboarding checklist counting a field no player reads
"Default Content" is persisted by the device route, snapshotted and restored by the settings layer,
offered in the device form in five languages — and read by nothing. Grep the whole tree and it
appears only in those places, the schema, and this checklist. It is absent from assemblePayload,
from every socket payload, and from all four players.
Counting it as "content assigned" therefore told the operator their screen was set up while the
screen itself went on showing "waiting for content" — the checklist confirming the one thing it
exists to confirm, incorrectly. It now counts only a playlist or a layout, both of which really do
put something on a display.
An existing test asserted the opposite ("any of the three ways of assigning counts"). It encoded the
same false premise, so it is replaced by one that pins the corrected behaviour along with the
evidence for it. The column and the form field are left alone — whether to implement or remove the
feature is a product decision, and this change only stops the checklist making a claim on its
behalf.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|
||
|
Claude
|
9ea1b5e07b |
Stop eight dashboard views reporting success for requests the server refused
Each of these views carries its own copy of a fetch helper ending in `.then(r => r.json())`. A 403, 404 or 500 body resolves as an ordinary value, so the surrounding try/catch is unreachable and every handler treats the failure as success. The shared client in api.js has always thrown on !res.ok; these local copies never did. Two concrete consequences, both of which tell the operator something untrue: - The layout editor renders a Delete button on built-in templates for everyone. The server returns 403. The handler shows "Layout deleted" and re-renders the list with the template still sitting there. - A rejected platform-role change in Admin shows "Role updated", and the revert that would put the dropdown back lives only in the dead catch — so the UI keeps displaying a value the server refused. The same control in Settings uses the throwing client, so the two pages disagree about whether the change happened. All eight now match the shared contract: reject on !ok with the server's own message, and treat 401 as session expiry the way api.js does. This makes previously-silent failures visible, which is the point — some of them will surface refusals that were always happening. The layout template Delete button, for instance, is now honestly reported as refused rather than falsely confirmed; whether that button should be shown at all is a separate question. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL |
||
|
Claude
|
f66c941c1d |
Stop the content edit dialog rewriting types it cannot represent
Opening Edit on a YouTube item and pressing Save Changes — with nothing else touched — turned it
into an MP4.
The type dropdown offers six fixed options and is rendered unconditionally. For video/youtube no
option matched, so the browser selected the first one, video/mp4. The save handler then reads the
select's value and sends it because it differs from the stored type:
const mimeType = overlay.querySelector('#editMimeType').value; // 'video/mp4'
if (mimeType !== contentItem.mime_type) updateData.mime_type = mimeType;
and the server stores what it is sent. mime_type is the renderer selector in every player, so the
item became an "MP4" whose source is a YouTube embed page: a dead slide on every screen in the
playlist. It could not be undone from the dialog either, because there is no video/youtube option to
set it back, and the YouTube-specific controls disappear once the type has changed.
The same applies to uploads the sniffer accepts but the list omits — the sniffer allows fifteen
types, the dropdown covers six — so .mov, .svg, .heic, .avif and .bmp were all rewritten the same
way.
The dialog now includes the item's actual type as a selected option whenever the fixed six cannot
express it, so opening and saving is a no-op and the type is never silently changed.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uaeo9MvzKoyXuN6ZsbhtkL
|