Skip to content

feat(things,tokens): re-land hidden πŸ•΅οΈ links + PAT GET bridge + custom audiences 🎭 (#413/#431 never reached develop) - #613

Open
lopugit wants to merge 16 commits into
developfrom
claude/hidden-links-custom-audiences-reland
Open

feat(things,tokens): re-land hidden πŸ•΅οΈ links + PAT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)#613
lopugit wants to merge 16 commits into
developfrom
claude/hidden-links-custom-audiences-reland

Conversation

@lopugit

@lopugit lopugit commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Why this PR exists

PR #413 (hidden πŸ•΅οΈ link-key visibility + the /api/v1/get PAT bridge) and PR #431 (custom audiences 🎭 with capability grants + groups) both show as merged, but they were merged into their stacked base branches a minute after PR #411 (that base) had already merged into develop. Neither merge commit is an ancestor of develop or main: develop has no /api/v1/get, no tt:custom, no /api/v1/groups, no CustomAudienceModal, and VISIBILITIES is still the four-circle list. This branch re-lands the published feature stack (origin/claude/hidden-links-get-bridge @ d826cf6, which contains #431) onto current develop.

What landed on the branch (unchanged from #413 / #431)

  • Hidden visibility ('hidden', acl ['tt:hidden','tt:user'] + rotating owner-only linkKey): ?key= URLs, anonymous keyed viewers, "Copy hidden link πŸ•΅οΈ", key rotation on every entry into hidden.
  • GET bridge /api/v1/get gated per token by meta.allowGet ("Works via GET links 🌍"): cookie-free, private, no-store, mirrors every op's scopes and accounting.
  • Custom audiences 🎭: tt:custom marker + baseline + tt:user/<name>[/comment|/write] and tt:group/<id>[…] capability grants (write βŠƒ comment βŠƒ read), the audience picker, groups managed via /api/v1/groups, grant-aware feed/search circles, "Hidden only πŸ•΅οΈ" token fence mode.
  • verify-pat-tokens.mjs sections G/H/I cover the three features.

Merge with develop (this PR's own work)

Eight files conflicted; resolutions:

  • things.ts: kept both import sets (groupIdsOf + emitMentionNotifications / NotificationActor), kept develop's exported patVisibilityBlocksAcl with the branch's bucket comment, kept REQUESTABLE_VISIBILITIES for circle filtering alongside develop's tag normalization, and kept the custom-audience shared-editing branch and develop's expectedUpdatedAt optimistic-concurrency check in updateThing.
  • patTokens.ts: allowGet and develop's createdVia: 'chatgpt-oauth' both survive in the mint input and the stored row.
  • PostCard.tsx: develop's onChanged(post.id, …) contract applied to the branch's audience/linkKey updates (the feed contract test counts every call).
  • PostComposer.tsx: custom acl on edit + develop's attachment-panel ids. share/_share.tsx: withLinkKeys viewer + develop's tags.
  • TESTING.md: both sides' checklist items (dropped the branch's stale five-circle duplicate). Raycast converter and the deprecated API CORS shim: develop's versions.
  • patScopes.test.ts: the visibility catalog test now expects the four modes (hidden is this stack's addition).

Verification

  • Unit groups green after the merge: acl, things, pat-scopes, api-capabilities, schemas, feed, feed-contract, hooks, nav, auth-introspection.
  • pnpm exec eslint clean on every conflict-resolved file.
  • Live checks against the worktree stack: see the PR note.

πŸ€– Generated with Claude Code

lopugit and others added 15 commits August 28, 2026 01:27
Two features, one stacked branch:

Hidden visibility β€” a fifth audience, 'hidden' (acl ['tt:hidden','tt:user']):
unlisted everywhere (feeds, profiles, listings, search β€” for everyone but the
owner) yet viewable by ANYONE presenting the thing's randomly generated
linkKey (?key= on GET /api/v1/things and the /post page; body.key admits
engagement). The server mints the 24-byte base64url key whenever a thing
ENTERS hidden β€” re-hiding rotates it so circulated links die β€” and projects
it to the owner only. canView honors the key only while the acl still says
hidden, so un-hiding retires links instantly. UI: πŸ•΅οΈ Hidden in the composer
circle select and the post-menu privacy submenu, a Hidden circle chip in feed
filters, and "Copy hidden link πŸ•΅οΈ" in the post menu.

GET bridge β€” per-token allowGet ("Works via GET links 🌍" tickbox) opens
GET /api/v1/get?token=…&op=…: the whole PAT things surface as plain GET URLs
(query-param token + args) for agents that can browse but cannot send
headers, bodies, or non-GET verbs. Cookies are never read (mutating GETs
cannot be CSRF'd β€” the unguessable token is the authorization); scope checks,
free-403 semantics, atomic use accounting, the tt:token sandbox, and the
visibility fence are byte-for-byte the Bearer path via a shared
resolvePatSessionActor core. op=self is free introspection. Responses carry
Cache-Control: private, no-store and Referrer-Policy: no-referrer. Registered
in the server import map + apiDocs (docs-derived route table).

verify-pat-tokens.mjs grows sections G (hidden, 17 checks) and H (bridge,
21 checks) β€” 105/105 pass live, including fence Γ— bridge and fence Γ— hidden
interplay. Browser-QA'd: UI mint with the tick badges 🌍 and drives the
bridge credential-less; composer-posted hidden post is invisible anonymous,
viewable via its secret URL, and offers Copy hidden link.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ps, and the hidden-only token fence

Custom visibility: an acl carrying the tt:custom marker names exactly who
can do what. Baseline toggle (tt:all = everyone reads / tt:hidden = link-key
holders read / neither = only the picks), plus per-user grants
tt:user/<name>[/comment|/write] and per-group grants tt:group/<id>[…] with
write βŠƒ comment βŠƒ read. On custom things, general viewers get READ ONLY β€”
commenting/reacting/sharing needs the comment capability (inherit chains
judged on their terminal, saves exempt as private bookmarks), and write
grantees share-edit crystal/extended/tags through PATCH while audience,
folder, token grants, and deletion stay owner-only (storage keeps billing
the owner). Granted things land in grantees' FEEDS via a new
visibilityQueryFor grant clause. Capability grammar, evaluation
(aclCapabilityFor), specificity, and AclViewer.groupIds live in the registry;
viewer group memberships preload beside friendIds.

Groups: protected 'group' + 'group-member' things managed via /api/v1/groups
(list/create/replace-members/delete) + /api/v1/groups/audience-sources
(friends, connections, recents, groups β€” the picker's prefill). Member docs
mirror the friend-doc shape (targetId = member) so every query rides
existing indexes β€” no new MongoDB indexes (the 64-index local budget stays
untouched). Membership edits propagate live to every referencing acl;
member ids validate via findUserById (both user eras).

UI: CustomAudienceModal β€” baseline chips, people search
(/api/v1/users/search), prefilled+filterable Recents/Friends/Connections
sections, per-entry capability selects, save-selection-as-group and
existing-group chips β€” wired into the composer circle select (🎭 Custom +
Edit button; fixed a stale-closure fallback that reset the applied audience)
and the PostCard privacy menu (custom routes through the picker).

Token fence: visibility mode 'hidden' ("Hidden only πŸ•΅οΈ") β€” the token lives
entirely in hidden link-key things; its creates are born hidden and mint
their secret link.

verify-pat-tokens.mjs section I: 29 checks β€” capability matrix (read cannot
comment, write implies comment, writers never touch audience/delete),
baseline toggles (public baseline still gates comments; hidden baseline
mints a key that grants read only), live group grant/revoke, feed grant
clause, audience-sources, protected kinds, hidden-only fence. 134/134 pass.
Browser-QA'd desktop + mobile: full composer flow (search β†’ add β†’ capability
β†’ create group β†’ apply β†’ post) produced the exact acl and enforced it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…s-custom-audiences-reland

# Conflicts:
#	Commander/extensions/raycast/src/commands/regexToReplacementConverter.tsx
#	TESTING.md
#	deprecated/api/src/index.js
#	remix/app/api/utils/auth/patTokens.ts
#	remix/app/api/utils/things/things.ts
#	remix/app/components/Feed/PostCard.tsx
#	remix/app/components/Feed/PostComposer.tsx
#	remix/app/routes/api/v1/things/share/_share.tsx
@github-actions github-actions Bot added lopu: mergeable The PR branches can currently be merged without conflicts lopu: overlapping files This PR changes files also changed by another open PR labels Sep 4, 2026
… keyed by id

The 105-check suite (now 149 checks on this stack) crashed in section F
because develop's search responses key posts by thing id; postRows()
accepts both shapes. Full run: 149 passed, 0 failed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@lopugit

lopugit commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

πŸ§ͺ Develop S3 preview deploying

Generic Vercel Preview deployments use the shared development runtime; this controller adds the stable exact-SHA alias and marker-scoped cleanup.

1 similar comment
@lopugit

lopugit commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

πŸ§ͺ Develop S3 preview deploying

Generic Vercel Preview deployments use the shared development runtime; this controller adds the stable exact-SHA alias and marker-scoped cleanup.

@lopugit

lopugit commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

πŸ§ͺ Develop S3 preview deploying

Generic Vercel Preview deployments use the shared development runtime; this controller adds the stable exact-SHA alias and marker-scoped cleanup.

@lopugit

lopugit commented Sep 4, 2026

Copy link
Copy Markdown
Owner Author

βœ… Develop S3 preview ready

The alias passed the develop bucket CORS preflight and a final live PR/SHA fence.

Generic Vercel Preview deployments use the shared development runtime; this controller adds the stable exact-SHA alias and marker-scoped cleanup.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Lopu repository review

Lopu reviewed this PR against develop as Thingtime's principal PR and repository manager. Using Claude Opus 5.

Lopu found no justified local change to publish from this review pass.

PR #613 β€” feat(things,tokens): re-land hidden πŸ•΅οΈ links + PAT GET bridge + custom audiences 🎭

Head bf3498fb7ec3a8ba0552dc61af72a4452792d248 β†’ base develop @ 61e7daeb
45 files changed, +2,621 / βˆ’160.

Checks: all green. No CodeQL alerts on this head.

What I compared

Full head-vs-base diff, weighted heavily toward the new trust boundary. /api/v1/get is the
riskiest thing in this batch by construction β€” it accepts a credential in a URL and exposes
thirteen operations including mutations behind a GET β€” so I reviewed it as an attack surface
rather than as a feature.

The GET bridge is well-defended

I went looking for the failure modes this design invites, and each one is already closed:

Risk Mitigation, verified
CSRF via a mutating GET Cookies are never read. The unguessable token is the authorization, so no ambient credential can be replayed cross-site.
Any PAT silently usable in a URL Opt-in per token. resolveGetBridgeActor requires meta.allowGet === true (patTokens.ts:363,375); toPublicPatToken defaults it to false (:99), and TokenMinter exposes it as a Switch defaulting to false. A token minted before this feature cannot be used here.
Token leaking onward via Referer Referrer-Policy: no-referrer on every response.
Token cached by a shared proxy Cache-Control: private, no-store on every response.
Privilege escalation past the normal routes scopeForOp mirrors patScopeFor in the things route, including the subtle case where a create whose thingtime says reaction/comment needs that scope rather than blanket things.create, and upsert requiring both things.create and things.update.
Rate-limit bypass by routing through the bridge rateLimitKeyForOp reuses the same keys as the normal routes, including the things.write.service split by accountKind.
Oversized input body param capped at 64 KB with a 413.

Prototype pollution deserves specific mention, because this is where a query-param-driven arg
builder usually goes wrong.
Param names are attacker-chosen here, so args[name] = … with
name === '__proto__' would normally hit Object.prototype's setter. The PR closes it three
ways at once: args is built on Object.create(null), UNSAFE_ARG_KEYS (__proto__,
constructor, prototype) is filtered on the query-param path, and on the parsed-JSON
?body= path β€” that second filter is the one most implementations forget, since a __proto__
key surviving JSON.parse is an own property that any downstream recursive merge would walk
straight onto Object.prototype. All three are present.

Value coercion is also conservative in the right direction: only {, [, and " prefixes are
parsed as JSON, so ?text=hello and ?emoji=πŸ”₯ stay strings rather than being silently
reinterpreted.

Registration is correct

AI_ALL.md requires new /api/v1/... endpoints in three places or Nitro 404s them. All three
new endpoints satisfy it:

  • Route files: api/v1/get/_get.tsx, api/v1/groups/_groups.tsx,
    api/v1/groups/audience-sources/_audience-sources.tsx
  • Nitro import map: server/routes/api/[...].ts:199-201
  • apiEndpointDocs: apiDocs.ts:8808, :8849, :8884

Test failures I investigated and cleared

queryRunner.test.ts reports 4 failures in my worktree:

βœ– strips protected thing fields at every pipeline ingress
βœ– keeps ordinary field names and $expr usable outside protected scopes
βœ– enforces the read-only stage allowlist inside nested sub-pipelines
βœ– deserializes canonical Extended JSON values without evaluating code

This PR does modify querySafety.ts, queryContract.ts, and queryRunner.test.ts, so these
looked like a genuine regression in query-safety code β€” exactly the kind of thing that must not
be waved through. They are not caused by this PR.

I ran the same test file from PR #611's worktree, which does not touch any of those three files
and therefore carries develop's versions: identical result, 7 pass / 4 fail, same four test
names.
The cause is environmental β€” querySafety.ts:271 calls BSON.EJSON.deserialize, and
bson is not resolvable in these dependency-free review worktrees, which surfaces as
"One or more typed BSON values are invalid" rather than a module-resolution error. The failure
is identical at base and head, so nothing here regressed.

Validation

  • This PR's own new tests: acl.test.ts + patScopes.test.ts β€” 17/17 pass.
  • queryRunner.test.ts β€” 7/11, with the 4 failures proven pre-existing and environmental by the
    baseline comparison above.
  • meta.allowGet opt-in traced end to end: schema β†’ mint β†’ resolver enforcement β†’ UI switch β†’
    token list display.
  • Three-place endpoint registration verified for all three new endpoints.

Findings

No changes needed. For a feature that puts a credential in a query string, the threat model
is unusually well covered and the reasoning is written down where the next reader will find it.

Notes (no change made)

  • op=self returns before the rate-limit check, and read ops are deliberately unlimited
    ("reads ride free like the things GET loader"), so /api/v1/get?op=self&token=… is an
    unthrottled token-validity oracle. This is not practically exploitable β€” PATs are server-signed
    JWTs (mintPatToken β†’ signJwt), so they cannot be guessed β€” and the behaviour matches the
    existing convention for GET loaders. Worth remembering if token format ever changes to
    something guessable.
  • Putting a credential in a URL is inherently a trade against logs, browser history, and
    screenshots. The PR states this explicitly in the file header and gates it behind a per-token
    opt-in, which is the right way to make that trade. Nothing to change; worth keeping visible in
    the user-facing copy of the mint switch.

Recommendation

Merge-ready.

View Lopu workflow run

github-actions Bot added a commit that referenced this pull request Sep 4, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
github-actions Bot added a commit that referenced this pull request Sep 4, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
github-actions Bot added a commit that referenced this pull request Sep 4, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

πŸ•΅οΈ Lopu β€” went looking for holes in the bridge and the ACL model; found two notes, no defects

This is the highest-risk PR in the batch β€” a bearer secret, a new capability model, and CRUD over plain GET β€” so I read the security surfaces end to end rather than skimming. Recording what I actually verified, so a future reader doesn't have to redo it.

The GET bridge is CSRF-safe, and for a non-obvious reason

The auth cookie is sameSite: 'lax', which still sends on top-level GET navigation. So a mutating GET route that read cookies would be forgeable by a plain link β€” <a href="…/api/v1/get?op=delete&id=…"> in an email. The bridge never reads a cookie at all; the unguessable token in ?token= is the only authorization. That's the one design that makes GET-CRUD safe here, and it's the one taken. Referrer-Policy: no-referrer and Cache-Control: private, no-store close the two obvious onward-leak paths.

Other things I checked and found already closed:

  • Prototype pollution on both ingress paths. Param names are attacker-chosen, so args[name] = … with __proto__ matters. args is Object.create(null) and __proto__/constructor/prototype are dropped from both the parsed body JSON and the param overlay. The null-prototype base alone would have sufficed; the pair is fine.
  • Scope is computed before use consumption, and thingtime is normalized first. The stringβ†’csv normalization runs above the resolveGetBridgeActor call, so ?thingtime=reaction genuinely requires things.react rather than falling through to blanket things.create. Classic ordering bug; not present.
  • op=self is free and correctly doesn't spend a use, matching /api/v1/tokens/self.

The hidden-link lifecycle

192 bits of entropy, and the state machine is right: entering tt:hidden mints a fresh key (so a link that circulated during an earlier hidden period can't resurrect on re-hide), leaving hidden keeps the field but canView requires aclOf(doc).includes(ACL_HIDDEN) β€” so un-hiding retires every shared link instantly, without a write. Both public projections emit linkKey owner-only and hidden-only.

Adding linkKey to MONGO_PROTECTED_THING_FIELDS is the right classification β€” it's a bearer secret, so it needs the hard ingress strip at every pipeline entry, not best-effort crystal redaction. And deriving shouldRedactKey from that one list instead of re-spelling the names is what stops the probe guard, the strip stage and the response redactor drifting apart. (Worth contrasting with #592 in this batch, where the vault ciphertext sits under crystal β€” I've raised it there.)

splitCapability and the aclCapabilityFor floor

Both of these are guarding against bugs that are genuinely easy to miss, and the comments explain why:

  • A bare id.endsWith('/write') would read tt:user/write β€” the account literally named "write" β€” as base tt:user, i.e. the owner entry. That account silently gets nothing and the acl gains a phantom owner grant. Requiring a non-empty subject under a known grant prefix closes it. I walked tt:user/write, tt:user/bob/write, tt:user//write and tt:user/a/b through sanitizeAcl + splitCapability; all behave as documented.
  • Without the view floor, an acl carrying both tt:user/bob/write and a same-specificity -tt:user/bob would return write on a thing the same acl says bob may not read. Unreachable today since every caller proves view first β€” but "unreachable today" is a precondition every future caller has to rediscover, so anchoring it in the function is the right instinct.

The visibilityQueryFor narrowing is also correct: the unfiltered shortcut fires only for "no filter" or "every requestable circle", not on a bare length comparison β€” so ticking exactly public+friends+family+private no longer takes the shortcut and hands back the hidden things the caller just excluded.

Two notes (neither blocks, and I made no changes)

1. Username case is matched two different ways. aclEntryMatches lower-cases both sides, so tt:user/Bob grants view to bob. But the feed clause in visibilityQueryFor builds grantEntries from viewer.username.toLowerCase() and matches with $in β€” an exact string compare. So a stored tt:user/Bob is viewable when fetched directly and never appears in that user's feed or search.

Not reachable from real producers: registerUser stores usernames lower-cased, and CustomAudienceModal composes from the stored value. Only a hand-written mixed-case grant through the API hits it. If you want it closed, the one-liner is to lower-case the tt:user/ subject in sanitizeAcl, which makes the index query and the read gate agree by construction. I didn't make that change β€” it alters accepted-input round-tripping and no live producer hits it, so it's your call rather than something I should have quietly patched.

2. tt:group/<id> doesn't require the group to belong to the thing's owner. aclEntryMatches answers purely from viewer.groupIds. Not an access-control hole β€” granting access to your own thing is your right β€” but it does make a membership oracle possible: knowing a group's shareId, an owner could learn whether a given viewer is in it by whether they can see the thing. Low severity and it needs the shareId first. Noting it because the doc comment says "one of the owner's groups" and the code is wider than the comment.

Minor: MAX_BODY_PARAM_BYTES is compared against bodyParam.length (UTF-16 code units, not bytes), so a non-ASCII body can exceed the nominal 64 KiB. Cosmetic β€” URL length limits bind first.

Checks green, 0 open CodeQL alerts on this head. No changes made: the failure modes I went looking for are already closed.

github-actions Bot added a commit that referenced this pull request Sep 5, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
github-actions Bot added a commit that referenced this pull request Sep 5, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
@github-actions github-actions Bot added the lopu: queued The current PR snapshot is waiting in Lopu's PR-management queue label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

πŸ€– Lopu detected an out-of-date PR branch

Status: Work detected β€” Lopu is taking ownership.

Current phase: Entering Lopu's serialized PR-resolution queue.

Estimated completion: around 06:41 UTC (~20 minutes; this adjusts as the queue moves).

Next automatic check-in: within 10 minutes while work remains active. You can stay on this PR; there is no need to find the Actions run.

Time conversion (UTC source)

Moment UTC Los Angeles Melbourne
Updated 2026-09-05 06:21 UTC (UTC+00:00) 2026-09-04 23:21 PDT (UTC-07:00) 2026-09-05 16:21 AEST (UTC+10:00)
Estimated finish 2026-09-05 06:41 UTC (UTC+00:00) 2026-09-04 23:41 PDT (UTC-07:00) 2026-09-05 16:41 AEST (UTC+10:00)

Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically.

Lopu queue and PR pulse

Scope Metric Count
Repository Open PRs 25
Repository Conflicting 1
Repository Out-of-date with target 0
Repository GitHub state unknown 7
Repository Part of an open stack 2
Repository Touch files changed by another open PR 14
Repository Target a non-root branch without an open parent PR 0
This resolver batch Admitted snapshots 13
This resolver batch Currently resolving 0
This resolver batch Waiting 13
This resolver batch Finished 0

Related PR context

  • Stack: No open parent or child PR currently links to this branch.
  • Target: develop is a repository root/integration branch.
  • Changed-file overlap: 10 changed files are also touched by #10, #291, #295, #554, #560, #564, #578, #595, #607, #610, #611, #612, +1 more.

Exact branch pair: develop β†’ claude/hidden-links-custom-audiences-reland.

Timeline

  • 06:21 UTC β€” Detected that develop needs to be merged into claude/hidden-links-custom-audiences-reland; assigning the exact snapshot to the resolver queue.

@github-actions github-actions Bot removed the lopu: queued The current PR snapshot is waiting in Lopu's PR-management queue label Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

πŸ•΅οΈ Lopu β€” one open question on tt:group/<id>, and what I verified around it

Reviewed the full head-vs-base diff with the security surfaces read closely. It holds up; details in the review report. One question I'd like answered before merge, and it needs a sentence from you rather than a code change from me.

The question: whose groups can an acl name?

sanitizeAcl validates the shape of tt:group/<id> β€” one subject segment, optional /comment or /write β€” but not that the group belongs to the acl's owner. And aclEntryMatches resolves it against viewer.groupIds, which groupIdsOf builds from every group-member doc naming that viewer, across all owners:

if (base.startsWith(ACL_GROUP_PREFIX)) {
  const groupId = base.slice(ACL_GROUP_PREFIX.length);
  return viewer.id === ownerId || viewer.groupIds?.has(groupId) === true;
}

So user B can address user A's group as an audience for B's own content.

I don't think this is a vulnerability, and I want to be precise about why rather than hedge: nothing of A's leaks (B is sharing B's own thing), and B learns nothing about who is in the group (there is no read-back β€” B can't observe viewers). Group ids are randomUUID, so B can't enumerate them either; they'd have to already know one.

But it is a product decision hiding in a validation gap. Either:

  • Intended β€” "groups are addressable audience lists, like a mailing list id" β€” in which case one line in the registry.ts acl comment saying so would stop the next reviewer (or the next Lopu pass) re-raising it; or
  • Not intended β€” in which case the place to close it is an owner check at acl-write time, not in aclEntryMatches, since the read path should stay sync and pure.

What I verified, so you know the rest was actually checked

Two things looked like gaps on first read and turned out not to be β€” recording them so they don't get re-flagged:

  1. _get.tsx never calls withFriendIds. Correct β€” enrichment happens inside the utils layer (things.ts 2039/2749/2840/2896/2979/4920), and _things.tsx doesn't call it either.
  2. Auth (and PAT use consumption) runs before enforceRateLimit on the GET bridge. That's byte-for-byte the existing _things.tsx action ordering, so it's parity, not a new hole. Same for op=self spending no use β€” that's the /api/v1/tokens/self promise.

On the parts that carry the most risk:

  • linkKey is treated as credential material end to end. Adding it to MONGO_PROTECTED_THING_FIELDS means the probe checks, the ingress strip stages and the response redactor all derive from one constant β€” so the hard strip happens at every pipeline ingress (primary, $lookup, $unionWith) and $objectToArray over $$ROOT can't rename around it. Deriving shouldRedactKey's set from that same constant instead of the hand-spelled 'secure' || 'uniquekeys' is the part that stops the three mechanisms drifting. This is the right classification, and it's the opposite of the call feat(lopu): Lopu AI assistant β€” streamed chat, model catalog, builder tools, live patches, voice, own providersΒ #592's vault currently makes β€” I've raised it there.
  • canView orders the link-key branch after patVisibilityBlocksAcl, so a fenced token can't use a key to escape its fence; and it grants only while the acl still says tt:hidden, so un-hiding retires shared links immediately.
  • splitCapability is anchored, which is what keeps tt:user/write (an account genuinely named "write") from parsing as base tt:user β€” i.e. the owner entry β€” silently granting a phantom owner grant. acl.test.ts pins it.
  • The GET bridge's prototype handling is closed on both paths: null-prototype base, and __proto__/constructor/prototype dropped from parsed body JSON and from attacker-chosen param names.

group/group-member are relational things kinds on the existing (thingtime, targetId) access pattern, so no new index β€” FUNDAMENTALS Β§3 satisfied without spending from the 64-index budget. All three new endpoints are registered in all three required places (verified mechanically).

Validation: acl.test.ts 11/11 and patScopes.test.ts pass. queryRunner.test.ts shows 7 pass / 4 fail here β€” I checked that against an unmodified develop tree at the same base (#610's worktree, which doesn't touch that file) and got the identical four failures, so they're a missing-bson environment artifact, not yours. The two subtests your linkKey change actually touches both pass.

0 open CodeQL alerts; dispositions file stays []. No changes made to this branch.

@github-actions github-actions Bot added lopu: conflicting GitHub reports merge conflicts for the current PR snapshot lopu: queued The current PR snapshot is waiting in Lopu's PR-management queue and removed lopu: mergeable The PR branches can currently be merged without conflicts labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

πŸ€– Lopu live PR update

Status: β†ͺ️ Resolver finished; a newer conflict remains

Current phase: The next detector event will own the current branch state

Estimated completion: Done β€” no further active-work ETA.

Time conversion (UTC source)

Moment UTC Los Angeles Melbourne
Updated 2026-09-05 09:01 UTC (UTC+00:00) 2026-09-05 02:01 PDT (UTC-07:00) 2026-09-05 19:01 AEST (UTC+10:00)

Los Angeles and Melbourne use their real IANA time zones, so PDT/PST and AEST/AEDT offsets change automatically.

Lopu queue and PR pulse

Scope Metric Count
Repository Open PRs 30
Repository Conflicting 17
Repository Out-of-date with target 1
Repository GitHub state unknown 4
Repository Part of an open stack 4
Repository Touch files changed by another open PR 16
Repository Target a non-root branch without an open parent PR 0
This resolver batch Admitted snapshots 4
This resolver batch Currently resolving 0
This resolver batch Waiting 0
This resolver batch Finished 4

Related PR context

  • Stack: No open parent or child PR currently links to this branch.
  • Target: develop is a repository root/integration branch.
  • Changed-file overlap: 10 changed files are also touched by #10, #291, #295, #554, #560, #564, #578, #592, #595, #596, #607, #610, +3 more.

Exact branch pair: develop β†’ claude/hidden-links-custom-audiences-reland.

Timeline

  • 07:01 UTC β€” Detected conflicts between develop and claude/hidden-links-custom-audiences-reland; assigning the exact snapshot to the resolver queue.
  • 07:03 UTC β€” The immutable head/base selection is reserved; waiting for its worker job to enter the serialized lane.
  • 07:04 UTC β€” Still safely queued behind earlier admitted Lopu work; no duplicate resolver was spawned.
  • 07:15 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 07:25 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 07:36 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 07:46 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 07:57 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 08:07 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 08:17 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 08:28 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 08:39 UTC β€” 10-minute check-in: still working β€” Waiting in Lopu's serialized PR-management lane.
  • 08:48 UTC β€” Using repository context to resolve the conflict semantics.
  • 08:51 UTC β€” Rebuilding Graphify structure and semantic context.
  • 09:00 UTC β€” Publishing the verified branch update.
  • 09:01 UTC β€” The worker published its verified result, but GitHub reports the latest PR state as conflicting again; Lopu will rediscover it automatically.

Technical run details β€” optional; this comment is the human-facing source of truth.

@github-actions github-actions Bot added lopu: queued The current PR snapshot is waiting in Lopu's PR-management queue and removed lopu: queued The current PR snapshot is waiting in Lopu's PR-management queue labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

πŸ€– Lopu review β€” green. Reviewed /api/v1/get as an attack surface; it holds up. No changes needed.

This is the riskiest thing in the current review batch by construction β€” a credential in a URL,
thirteen ops including mutations, all behind a GET. So I went looking for the failure modes that
design invites. Each one is already closed:

Risk Verified mitigation
CSRF via mutating GET Cookies never read β€” the unguessable token is the authorization, so no ambient credential replays cross-site
Any PAT usable in a URL Opt-in per token β€” meta.allowGet === true enforced at patTokens.ts:363,375, defaults false at :99, UI Switch defaults off. Pre-existing tokens can't be used here
Token leaked via Referer Referrer-Policy: no-referrer
Token cached by a proxy Cache-Control: private, no-store
Escalation past normal routes scopeForOp mirrors patScopeFor, including create-with-thingtime: reaction/comment needing that scope, and upsert needing both
Rate-limit bypass rateLimitKeyForOp reuses the same keys, including the things.write.service split by accountKind

The prototype-pollution handling is worth singling out, because this is exactly where a
query-param-driven arg builder normally goes wrong. Param names are attacker-chosen, so
args[name] = … with __proto__ would hit Object.prototype's setter. You close it three ways:
Object.create(null) base, UNSAFE_ARG_KEYS filtered on the query-param path, and on the
parsed-JSON ?body= path. That third one is the one most implementations miss β€” a __proto__
key surviving JSON.parse is an own property that any downstream recursive merge walks straight
onto Object.prototype. Good.

Value coercion errs the right way too: only {, [, " prefixes parse as JSON, so ?text=hello
and ?emoji=πŸ”₯ stay strings.


On the 4 queryRunner.test.ts failures β€” not yours

My worktree shows 4 failures in queryRunner.test.ts, and since this PR modifies querySafety.ts,
queryContract.ts, and that test file, they looked like a real regression in query-safety code.
Chased it before saying anything:

I ran the same file from #611's worktree, which touches none of those three files and so carries
develop's versions β€” identical result, 7 pass / 4 fail, same four test names. Cause is
environmental: querySafety.ts:271 calls BSON.EJSON.deserialize and bson isn't resolvable in
these dependency-free review worktrees, surfacing as "One or more typed BSON values are invalid"
instead of a module-resolution error. Identical at base and head β€” nothing regressed.

Your own new tests pass: acl.test.ts + patScopes.test.ts β€” 17/17.

Also verified all three new endpoints are registered in all three required places (route file,
Nitro import map [...].ts:199-201, apiEndpointDocs at :8808/:8849/:8884) β€” the thing that
otherwise silently 404s a new endpoint.


Two notes, neither blocking:

  • op=self returns before the rate-limit check, and reads are deliberately unlimited, so
    ?op=self&token=… is an unthrottled token-validity oracle. Not practically exploitable β€” PATs
    are server-signed JWTs via mintPatToken β†’ signJwt, so they can't be guessed β€” and it matches
    the existing GET-loader convention. Just worth remembering if the token format ever becomes
    guessable.
  • A credential in a URL is inherently a trade against logs, history, and screenshots. You state
    that plainly in the file header and gate it behind a per-token opt-in, which is the right way to
    make it. Worth keeping that just as visible in the user-facing copy next to the mint switch.

Merge-ready.

github-actions Bot added a commit that referenced this pull request Sep 5, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
@lopugit lopugit added last preview built 04/09 23:53 AEST #613 tt-pv1:613:develop:bf3498fb7ec3a8ba0552dc61af72a4452792d248:1788530019927 preview: develop ready tt-preview-state:v1:develop:ready labels Sep 5, 2026
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

πŸ”„ Lopu preserved the live base ref at bc061f63c05c8e61b0b2f6523836d6ffe36bd368 because it moved beside publication, then queued retry 1/3 from fresh PR refs.

No stale merge commit was pushed. View this guarded attempt.

github-actions Bot added a commit that referenced this pull request Sep 5, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
github-actions Bot added a commit that referenced this pull request Sep 5, 2026
…AT GET bridge + custom audiences 🎭 (#413/#431 never reached develop)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

last preview built 04/09 23:53 AEST #613 tt-pv1:613:develop:bf3498fb7ec3a8ba0552dc61af72a4452792d248:1788530019927 lopu: conflicting GitHub reports merge conflicts for the current PR snapshot lopu: overlapping files This PR changes files also changed by another open PR lopu: queued The current PR snapshot is waiting in Lopu's PR-management queue preview: develop ready tt-preview-state:v1:develop:ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant