Skip to content

Add a unified Google Drive Folder resource - #440

Open
ndisidore wants to merge 22 commits into
mainfrom
feat/gk-google-drive-folder
Open

ndisidore wants to merge 22 commits into
mainfrom
feat/gk-google-drive-folder

Conversation

@ndisidore

@ndisidore ndisidore commented Sep 3, 2026

Copy link
Copy Markdown
Member

Replaces the separate Shared Drive resource with one Google Drive Folder resource, read-only.
One selection covers an ordinary folder in My Drive, a folder someone else shared with you,
or a subfolder inside a shared drive.

A folder binding sees its direct children only, not everything beneath it: listing and
provider-side search are scoped to that one folder, and a child folder is reached by opening
it as its own capability rather than by recursive traversal.

It requests the same three read-only scopes as the account and exact-file resources. The picker
is one corpora=allDrives search, which finds every folder kind above without an account-wide
grant. It returns one provider page of suggestions, not a complete list, so a shared drive's own
root or a folder that page misses is still connectable by its URL, which prefills the form. An
incomplete search from Drive fails rather than looking complete.

Folder-derived Doc and Sheet sessions re-prove ancestry on every method call, not only
at open, so a file moved out of the folder stops answering through a session that is
already open. Drive has no ancestry-plus-content transaction, so a move landing after
the final check still returns and the next read denies.

2026-09-03_18-59
Devin Review

@github-actions github-actions Bot added the gatekeeper Changes to a gatekeeper integration label Sep 3, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch from 24a03b3 to 7256fd6 Compare September 3, 2026 22:56
@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown

Preview: pr440-feat-gk-googl-62f8970b

https://pr440-feat-gk-googl-62f8970b-router.cloudflare-os-previews.workers.dev

Dashboard · deleted when this PR closes

@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch from 7256fd6 to 9ae99a3 Compare September 4, 2026 16:16
devin-ai-integration[bot]

This comment was marked as resolved.

@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch from 9ae99a3 to 59d33f8 Compare September 8, 2026 17:37
devin-ai-integration[bot]

This comment was marked as resolved.

@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch from 59d33f8 to 0bc4323 Compare September 8, 2026 19:52
@cloudflare cloudflare deleted a comment from ask-bonk Bot Sep 8, 2026
@cloudflare cloudflare deleted a comment from ask-bonk Bot Sep 8, 2026
@cloudflare cloudflare deleted a comment from ask-bonk Bot Sep 8, 2026
@cloudflare cloudflare deleted a comment from ask-bonk Bot Sep 8, 2026
@github-actions github-actions Bot added workshop/frontend Changes to the Workshop frontend workshop/shared Changes to shared Workshop APIs labels Sep 16, 2026
github-advanced-security[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Sep 16, 2026

Copy link
Copy Markdown

Posted 1 actionable inline finding.

github run

@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch from 98d687a to 7b78ae3 Compare September 16, 2026 20:40
devin-ai-integration[bot]

This comment was marked as resolved.

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Sep 16, 2026

Copy link
Copy Markdown

Posted 2 actionable inline findings.

github run

@ndisidore ndisidore changed the title Add a folder-scoped Google Drive resource Add a unified Google Drive Folder resource Sep 16, 2026
devin-ai-integration[bot]

This comment was marked as resolved.

@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch 2 times, most recently from 6fec054 to 86814d2 Compare September 17, 2026 00:05
@ndisidore

Copy link
Copy Markdown
Member Author

@ask-bonk review this

ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Sep 17, 2026

Copy link
Copy Markdown

Posted 1 actionable inline finding.

github run

Confines the existing read-only GoogleDriveSession API to one selected
folder's current descendant subtree, for both My Drive folders and
ordinary subfolders inside a shared drive. A shared drive's own root
stays with the Shared Drive resource.

Drive v3 offers no folder corpus, no folder-scoped token, and no
recursive ancestor predicate -- `'<id>' in parents` means direct
children only -- so membership is proved in the gatekeeper.
drive-folder-scope.ts walks each candidate's parent chain upward through
freshly batched files.get metadata, level by level, admitting only
acyclic same-domain chains of live folders that reach the root within
Drive's 101-hop nesting limit, then re-reads every node on the surviving
paths immediately before disclosure. Nothing is cached across the
operation that proved it: a hierarchy change rotates no credential and
bumps no cache generation.

The grant identity is an internal `/_resource/folder/:folderId` path
rather than the natural browser URL, because `/drive/folders/:driveId`
is already the shared drive's permanent identity and its pattern leaves
the query component wildcard -- a query-qualified variant would match
both resources and make resource selection order-dependent. describe()
still reports the natural URL for the UI.

Folder-derived Doc and Sheet children now revalidate: DriveSessionCore
.nativeRead() re-proves ancestry and the exact native MIME type before
the provider is contacted, re-checks the proved chain before any
approval, and discards the fetched value if either fails. Drive has no
ancestry-plus-content transaction, so a move landing after that final
check still returns; the next read denies, and the method says so.

CursorPager's authorize() now takes `exhausted` and its page budget is
maxProviderPagesPerCall: reaching the bound authorizes the empty
nonterminal page and returns [] rather than throwing, so a folder cursor
can filter a whole page without claiming there are no results. Only an
exhausted cursor reaches the empty-search path, which now fences
observer admission through ObserverTracker.prepareWithheld() -- a
withheld read registers no tracked set, so addObserver would otherwise
verify a candidate against nothing. The Drive observer denial no longer
names a file ID.
startResourceConfigurator() previously returned only iframe HTML plus a
gatekeeper-defined capability, so a gatekeeper had no way to say that
selecting a resource first needs a wider grant on the connected account.
The sandboxed frame cannot ask for one itself: it runs under
sandbox="allow-scripts" with no popup or top-navigation privileges, and
widening that sandbox would weaken every configurator to serve one.

ResourceConfiguratorFrame now carries an optional authorization action:
display copy plus a capability that prepares the consent attempt and
returns its URL, or omits the URL when the authority is already held.
The gatekeeper keeps the policy -- what scope, what nonce, what callback
-- and Workshop contributes only the trusted user gesture. No arbitrary
scope strings cross the boundary, and the field is optional, so every
existing gatekeeper configurator is unaffected.
ResourceConfiguratorHost now wraps the sandboxed configurator and, when
the frame declares one, renders the authorization control itself. The
popup is opened blank during the click and navigated after the
gatekeeper answers, because a window opened after an awaited RPC is
blocked by default. popup.opener is cleared, and only an http(s) URL
with no embedded credentials is navigated to; anything else closes the
window and reports a failure rather than following it.

Each action is keyed by frameKey, so switching account or resource while
a request is in flight invalidates that request generation and closes
its blank window -- a late answer cannot navigate a popup that now
belongs to a different account or resource. Unmount does the same.

A frame now owns two RPC capabilities, so disposeConfiguratorFrame()
releases both and is used by both configurator owners on replacement,
unmount, and late completion; the authorization stub is released even
when disposing the UI stub throws. ResourceConfiguratorHost.test.tsx
covers the blocked popup, the valid and credential-bearing URLs, the
already-granted answer, the fenced late result, and disposal.
One Google Drive Folder resource now covers both an ordinary folder and
a Workspace Shared Drive's root, so the separate Shared Drive resource
and its picker are gone. The identity is the natural browser URL,
https://drive.google.com/drive/folders/<folderId>, which is
unambiguous once no second resource claims that path, and the folder's
grant stays the narrow drive.metadata.readonly, documents.readonly and
spreadsheets.readonly set.

The capability is positioned rather than recursive.
GoogleDriveFolderSession lists and searches only the current folder's
direct children, and openFolder() appends one validated direct-child
edge to a new, independently disposable capability, leaving the parent
untouched. Both list() and search() carry a direct-parent predicate
into the provider request -- corpora=drive for a shared-drive root,
corpora=user otherwise -- so Google's own full-text index answers
within the folder instead of a broad search being filtered afterwards.
That replaces the upward ancestry proofs: every operation revalidates
the root and the root-to-current path, and observations are typed as
files or listable folders so a remembered folder unit must still be a
live listable folder.

Complete shared-drive discovery cannot be narrowed -- Google accepts
nothing smaller than drive.readonly for drives.list -- so it is an
optional, user-initiated expansion on the account rather than part of
any folder's grant. The stored OAuth flow carries a requestDriveReadonly
flag, which adds only that scope and is never recorded as resource
consent; a reconnect preserves an expansion the account already made.
listSharedDrives() checks the stored scope before calling the provider,
and the picker's Workspace Shared Drives source is offered alongside
Folders, which lists every folder the account can list children of,
including ones another person shared with it.
A folder session refused every out-of-scope direct lookup through
#authorizeWithheld, which commits the observer-withheld marker and makes
every future addObserver() fail. The trigger was ordinary use rather
than probing: a user pastes a link to a doc that lives one subfolder
down, the agent opens it by ID, and the binding is silently unshareable
from then on with no way to clear it.

Only one of the two refusals answers an owner-relative question.
#tryFetchFile collapses 403 and 404 to undefined, so an absent file
means "this account cannot see it" -- a negative a collaborator might
not share, which stays fenced. Whether a visible file is a direct child
of the positioned folder is objective, so refusing it discloses nothing
and now leaves admission open. Both paths still raise the same
outsideScope() error, so the caller cannot tell them apart.

The move-during-read test correspondingly asserts the stronger claim it
was always about: the moved file stays visible, so the content reaches
neither the approval queue nor the caller and nothing is authorized.
The trusted authorization control owns popup state, an unmount cleanup
and the whole OAuth hand-off, which the frontend conventions say earns a
file of its own; it sat inline in the configurator host. Its tests move
with it, leaving the host suite the cases that are about host wiring.

Its popup now opens under a stable window name. The account holds a
single pending OAuth flow, so a second click used to open a second tab
and strand the first on a superseded nonce; naming the window makes the
repeat click renavigate the same tab instead. Polling popup.closed to
re-enable the button was the alternative, but a background tab left open
would then wedge the control disabled long after its flow died.

Both hosts clear a superseded frame from an effect, so one committed
render could pair a newly chosen account with the previous account's
frame, and acting on it would configure or authorize the wrong account.
The host now takes the frame state plus the current selection and
resolves them through one guard, so no call site can forget the check.
GatekeeperModal already made the comparison at submit time in two
places; both route through the same helper now, and its duplicate local
state type is gone. BlueprintLandingPage records the account and
resource pattern its frame state never carried.
DriveFolderSessionCore was added as a copy of DriveSessionCore, leaving
two implementations of the staging, fencing and page-authorization
rules. They had already drifted: #authorizeFiles never discarded a
staged check when the approval was refused while its twin did, which is
harmless only because prepareObservation returns no discard today. Both
cores now extend DriveCoreBase, which owns authorizeUnits,
authorizeWithheld, tryFetchFile and the per-page rule, parameterized by
the revalidation and base units a moving scope adds. The empty-search
refusal has one definition rather than two that a reader has to diff by
eye.

Several facts had spread across layers on the way in. The live-listable
folder triple was written out six times in three files, and two of those
asserted only two of its three conditions; it is now
isListableFolderNode and isListableFolderFile beside the MIME constant
that defines it. The 403-or-404 "invisible to this account" rule
collapses into isFileInvisible, so the single-get paths cannot diverge
from what the batch path treats as account-wide. checkObservations
carried a second hand-written field mask feeding the same parser and now
sends DRIVE_SCOPE_NODE_FIELDS. oauthScopeCovers spelled its scope
implications as branches over inline literals, where a typo reads as an
ungranted resource and silently hides a configurator; it is a
SCOPE_COVERED_BY table so the next readonly pair is a row.

Four pieces of state or code turned out unreachable. driveFileToEntry
took a rootId no call site passed, so its documented rule withholding
the bound folder's own parent never ran. DriveApi.getDrive lost both
production callers when folders and shared drives merged.
FolderLocation.rootId is always folderIds[0] and was validated to be, so
it was an invariant to maintain rather than a fact to store. The optional
NativeRead parameters let a future opener construct a Drive-derived
session that silently skips revalidation, so they are required now and
the one binding that genuinely has nothing to revalidate says so.

The rest is local. GoogleDriveSessionImpl had three copies of
dup-the-queue-and-dispose-on-throw and recovered folder-ness with an
instanceof after building a core to ask; one #withQueue and the scope
kind it was re-deriving replace both. DriveSessionCore.nativeRead
captured this to reach one field, pinning the core's api, scope and
observer hooks for a native session's whole life, and now passes the
function. The trusted authorization control tracked one in-flight
request through two refs that had to agree on four exit paths, and told
every gatekeeper's user to retry a shared-drive selector.
The folder picker had two sources. **Folders** searched `corpora=user`;
**Workspace Shared Drives** enumerated `drives.list`, which Google offers
under no scope narrower than account-wide `drive.readonly` -- so listing
shared-drive roots cost an optional account-level expansion, carried by a
trusted authorization card outside the iframe and a whole reconnect flow
whose only purpose was widening that one grant.

Search `corpora=allDrives` instead. It spans My Drive, "Shared with me",
and every shared drive the account is a member of, and `files.list`
accepts it under the `drive.metadata.readonly` the folder resource
already requires. One field, one request, no second consent, and a
shared-drive root still reachable when Google returns it as a folder.

What that buys is worth stating plainly: the picker is now an
interactive search over one provider page, not an enumeration. It never
promised completeness in a useful sense anyway -- the old Folders source
was already one page -- and a known root that does not surface is still
connectable through the concrete folder URL, which opens the selector
prefilled. So no traversal pager, and no silent bounded collection
pretending to be exhaustive.

`allDrives` introduces one failure the single-corpus searches could not
have: Drive may answer `incompleteSearch: true`, a partial result that is
otherwise indistinguishable from a complete one. Reject it at the
provider boundary rather than inventing a warning envelope no caller
would be required to read. A normal `nextPageToken` is not that, and must
not be mistaken for it.

Positioned folder sessions are untouched: a bound shared-drive folder
still navigates its own `corpora=drive`, now pinned by a test, so
discovery cannot leak into a binding's boundary.

`drive.readonly` survives only as a private row in `SCOPE_COVERED_BY`,
where it truthfully subsumes the narrow requirements an account may
already hold more than. Nothing requests it.
Nothing needs it any more. The Google folder picker was its only
producer, and it now finds shared-drive folders under the scopes its
resource already requires, so the whole vertical slice -- an optional
`ResourceConfiguratorAuthorization` on the frame, an optional
`GatekeeperUser.requestResourceAuthorization()`, and the trusted
frontend control that ran it as a nonce-carrying connect flow -- is
authority with no caller. Kernel surface that exists for one deleted
feature is surface that will be reached for by the next one.

This also reverts the host refactor that came with it: frame identity
and disposal move back to each owner, where they were before. Keeping
the refactor would leave `GatekeeperModal`, `BlueprintLandingPage`, and
`ResourceConfiguratorHost` rewritten in a change that no longer has any
reason to touch them, and the pre-existing per-owner behaviour is
sufficient for the surface that remains.

`ResourceConfiguratorFrame` keeps its name and returns to being the
plain `GatekeeperUiFrame` alias every gatekeeper already references.
`workshop-shared`, `workshop-backend`, and `workshop-frontend` are now
identical to the merge base, so the Drive folder feature carries no
external-package diff at all. The normal resource-consent path --
`ensureResources`, `reconnectAccount`, staged restore/commit,
`completeConnectHandoff` -- is untouched throughout.
`prepareWithheld` writes its in-flight marker before asking for approval
precisely so an activation that dies mid-read leaves admission closed
rather than open. The catch in `authorizeWithheld` then discarded that
marker for *any* `authorizeObservation` failure, which undoes the thing
the ordering was for: the overseer can record the observation and lose
the response, and a deleted marker lets `addObserver` admit a later
collaborator with nothing to verify them against.

Only a marked refusal proves nothing was recorded. Everything else is an
unknown outcome, and an unknown outcome reaches the same durable state as
a commit -- the fence stands for good. That is already the rule in
`gatekeeper-kit`, where `prepareWithheld` gives `commit` and `abandon`
the same implementation and reclaims state only for
`isObservationRefused`, so reuse the helper rather than keep a second
answer to one question.

Worth knowing while reading this: nothing marks refusals yet, so today
every failure takes the unknown path and `discard` is unreachable. That
is the fail-closed direction, and the branch is here so it stays correct
when the overseer starts marking them.
`#cursor` revalidated the saved root-to-current path before returning the
pager, to learn which corpus to search. Nothing had authorized a read at
that point, so `list()` and `search()` answered whether the path is still
visible, listable and connected without an approval-queue entry or any
observer bookkeeping -- calling either and never paging was enough, and
repeating it is an unlogged change monitor over the bound folder.

Every other method on this session authorizes before it returns, and
`#requireDirectFile` fences an invisible probe outright, so this was the
one path that read first and audited later.

`fetchPage` already revalidates the path before each page, so take the
corpus from that read instead and keep the cursor synchronous, as its
account-scoped sibling has always been. The moved-drive check needs a
baseline rather than a pre-read: the first revalidation records the drive
it found and every later one compares against it, which is the same
guarantee one page later.
`#assertParent` used to refuse only a non-folder, which is objective: any
observer who can read the file's metadata can reach the same answer, so
recording a plain file unit was honest. This branch added
`canListChildren !== true` to the same condition without changing the
unit, and that half is owner-relative. A file observation proves metadata
access alone, so a collaborator who can see the folder but cannot list it
passed the backward check vacuously -- inheriting either the owner's
successful cursor or the refusal, on a folder they have no right to
enumerate.

Take the shape `openFolder` already uses for the identical predicate: a
live folder this account cannot list is owner-relative and takes the
withheld fence; a non-folder or a trashed one stays an objective file
disclosure, so a probe of some unrelated ID cannot latch sharing closed;
and a parent that passes is recorded as `{kind: "folder"}`, which
`checkObservations` verifies with `isListableFolderNode` rather than bare
reachability. That last part also covers the empty-listing case, where
the page observation carries no unit of its own.

Folding the trashed check in comes free with `isListableFolderFile`, and
matches the root and direct-child validators.
`normalizeSearch` throws "Drive search requires at least one filter", and
this branch condensed the sentence that said so out of the agent-facing
docs. Every `DriveSearchQuery` field is optional, so `search({})` and
`search({order: "nameAsc"})` type-check and then fail at runtime -- and
an order-only call is the obvious way to ask for a sorted listing, so an
agent reads the types, writes it, and burns a turn on the throw. The
sibling constraint on `order` survived on its own field; only this one
went.

`drive-types.txt` is what `getTypeScriptTypes()` ships, so the JSDoc here
is the whole contract an agent gets. Restore the requirement, restore the
note that omitting `order` keeps Drive's relevance ordering for full-text
search, and state both on the folder session too: it runs the same
`normalizeSearch` and withholds the same empty result, and documented
neither.
`GoogleDriveSessionImpl` implements both Drive session interfaces, so the
signatures `@validateRpc()` builds validators from are the account ones:
`DriveListOptions` and `DriveSearchQuery`, both carrying
`directParentId`. A folder capability advertises
`GoogleDriveFolderSession`, whose query types omit that field, and got
handed it anyway.

`list` discarded it. `search` did not: it satisfied `normalizeSearch`'s
required-filter check, so `search({directParentId: "decoy"})` ran where
`search({})` correctly throws, and while `#cursor` overrides the field
for the provider call, the same object reaches `pageAuthorizer` and
`queryClauses` renders `parent decoy` into the approval description --
indistinguishable from the genuine parent filter an account-scope search
shows there. The units and the provider query stayed correct, so nothing
escaped the folder; a gadget could put a chosen id into its own audit
line.

Take the folder query types on the folder core's own methods and refuse a
`directParentId` that arrives regardless. Refusing rather than stripping:
the published type says the field does not exist, so a call carrying it
is invalid, and silently dropping invalid input is how `list` hid this in
the first place. Separately validated session classes would make the
validated surface exact without a guard, but that means a facade per core
and widening the Durable Object's declared `Gatekeeper<GoogleDriveSession>`
to a union -- more wire surface churn than this earns.
`#exactFileCursor` drops a trashed bound file, so `list()` drains to
`null` on an exact-file binding whose file has been binned -- while
`getEntry()` still returns it, as its own doc says. The behaviour is
deliberate and pinned by two tests, and it is the same rule
`buildDriveQuery` applies to every other listing through its constant
`trashed = false`. What was wrong is the exported prose: this branch
condensed away the paragraph that used to state it, leaving `list()`
promising "the one exact-file entry" flatly, with the escape hatch
documented on one side only.

That makes three runtime constraints lost to the same condensation, after
the search filter requirement and the relevance-order note. So restore
the canonical statement on `DriveEntry`, where it covers both sessions at
once, and take `list()`'s undocumented throw with it -- `directParentId`
has to name a folder this account can list, and the refusal moved twice
on this branch without ever being written down.

I checked the rest of what the condensation dropped rather than assume:
the cursor's `[]`-versus-`null` contract survived and reads better than
it did, `fullTextContains`'s reach and `getEntry`'s bound-ID restriction
are intact, and the shared-drive scope paragraph went with the feature it
described. Two items, not ten.

Also note the one Drive URL shape that cannot be connected, since the
failure is a bare 404 at connect time with nothing to read it against:
`?resourcekey=` is dropped, which only matters for a link-access user of
an item shared by link before September 2021. Support means carrying
per-file keys through every request including batch subrequests, so it is
its own change; the README now says so rather than leaving the 404
unexplained.
A folder inside a shared drive can be shared directly with someone who
is not a member of that drive -- Google documents this as a file access
permission, deliberately contrasted with member permissions, and parent
permissions propagate down to children. Such a folder passes
`readFolderRoot`: `files.get` reports its `driveId` and
`canListChildren: true`, so the binding connects. Then the folder cursor
sends `corpora=drive&driveId=...`, which is an access of the shared drive
itself, and Drive answers 403 `teamDriveMembershipRequired`. Browsing is
dead while `getEntry`, `openFolder` and native opens keep working, since
those are all `files.get`.

The refusal is loud, which is the one mercy here: `fetchPage` sits
outside `#nextPage`'s only try, so the error lands before `buildEntries`
and before `authorize` -- nothing is observed and the withheld fence is
not latched.

This commit does not fix it, it stops the failure being a bare `403`.
Switching to the user corpus is the obvious-looking repair and it is a
trap: Google defines that corpus as My Drive items the account created or
opened plus items shared directly with it in "Shared with me", with no
shared-drive clause at all, so a *member* browsing a team folder would
lose the listing entirely, and a non-member's children are inherited
rather than directly shared, so they likely would not appear either. An
empty page is not inert on this path -- `search()` runs with
`denyEmptySearch`, so it latches the observer fence for good. `allDrives`
is no better: it too is scoped to drives the user is a member of, and it
is the corpus Google names as the source of `incompleteSearch`, which
this client hard-fails.

So the real repair is to refuse at connect, or to detect membership, and
it wants a live non-member account to design against. Until then the
error says which drive fact is missing, and the README stops implying the
corpus rule always works. Its account-binding paragraph also repeated a
looser claim about `corpora=user` than Google's own definition supports;
that now matches the documented wording.
`#getSnapshot` published its promise to `#snapshot` synchronously, and
the folder binding's scope check straddles the fetch it wraps: precheck,
fetch, postcheck. So a read refused by the postcheck still left its
revision in the cache, and `#nextSnapshot` returns a cached revision
inside `DOC_SNAPSHOT_TTL_MS` without contacting Docs at all. Move a
document out between the precheck and the fetch, edit it, move it back
inside ten seconds, and the retry is served the revision captured while
the document was outside -- certified as an in-scope read by an approval
record that says so. The README already promised the opposite: "if the
file moves meanwhile, the fetched value is discarded."

Fold both snapshot callers into one `#readSnapshot` that owns the
rollback, so there is a single place where a refused read and its cache
entry are reconciled. An identity check guards the rollback, leaving
alone a snapshot a concurrent read has already replaced, and any failure
reclaims it rather than only a membership one -- the distinction buys
nothing and an unrelated refusal costs one refetch.

Two properties this had to keep, and the first one I broke on the way
past. `NativeRead` takes a thunk precisely so the precheck can refuse
before the provider is contacted, and starting the snapshot chain eagerly
defeated that: every refused read went on to hit the Docs API, which
`refuses every Doc read after the document leaves the subtree` caught.
The chain now starts inside the thunk. Second, the assignment stays
synchronous within that thunk, so concurrent reads still chain onto one
fetch instead of racing to overwrite each other.

Scope is folder-derived Doc sessions alone: the editable Doc session and
every account or exact-file binding read through `unguardedNativeRead`
and have no postcheck to fail, and the Sheets session caches nothing.
Trimming the supplied parent brought `list()` in line with
`buildDriveQuery` and `normalizeSearch`, and took a refusal with it. At
merge base a whitespace id was truthy, so it reached `#assertParent`,
`files.get` answered 404, and the call failed. Now it trims to empty,
skips the check and the parent predicate, and returns an account-wide
listing: the caller asked to narrow to one folder and silently got
everything. `search()` has always done this when another filter carried
the required-filter check, so that half is not a regression, only the
same mistake.

Nothing escapes the binding -- an account binding may list its account,
and the approval description says "the connected Drive account" with no
parent clause, so the audit is accurate about what happened. The damage
is downstream, in an agent reasoning over the whole account as though it
were one folder's children.

Hence a refusal rather than a wider trim. A blank name or full-text
filter matches more rows within the scope the caller already has; a blank
parent returns rows from outside the folder they named, which is a
different kind of mistake and the reason this one field gets the check.
Both readers share it, since `normalizeSearch` is where the search path
trims.
Rolling the snapshot back after a refusal cannot work, and the identity
check I added made that look handled. A read that chained onto the
refused revision already holds it: `#nextSnapshot` returns the same
object inside the TTL, and the refused fetch itself resolved -- only the
guard around it failed -- so the sibling read publishes its own promise,
the identity check no longer matches, and the revision is disclosed by
whichever read passes its own postcheck.

Tainting the revision is equally too late, for the same reason: by the
time the refusal is known, the sibling is past its fetch. And publishing
only on success is not available -- `fetches the document once for
concurrent reads` pins that reads issued together share one revision,
because otherwise they observe different documents and the later response
can be the older one.

So split the one field by what it is for. `#inFlight` shares a fetch that
is still running, which is what that test is about and is safe on its own
terms: each joiner brackets the shared fetch with its own checks, exactly
as a lone read does. `#approved` holds only a revision a completed check
approved, and is the only thing a later read may reuse. A refused read
assigns nothing, so there is no state left to roll back, taint, or reason
about. `#nextSnapshot` now reads a plain value and loses its
await-and-swallow.

The precise interleaving is closed by construction rather than by a test:
reproducing it needs a move landing between two postchecks, and the
provider stub has no seam for ordering Drive reads against each other.
The new test locks what is checkable -- reads sharing one fetch, refused
together, leave nothing reusable -- and passes before this change too,
so it is a guard against regression rather than proof of the fix.
@ndisidore
ndisidore force-pushed the feat/gk-google-drive-folder branch from 662969c to 644e01d Compare September 18, 2026 14:40
ask-bonk[bot]

This comment was marked as resolved.

@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown

Posted 3 actionable inline findings.

github run

@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown
  • Medium packages/gatekeeper-google/src/drive-session.ts:634: A rejected Doc/Sheet fetch bypasses post-scope validation and authorizeObservation(). For example, probing a worksheet name via readRange() reveals existence through success versus HTTP 400 without recording an observation. Authorize failures before rethrowing them.

github run

`#inFlight` clears when the fetch settles, not when the guard around it
finishes, so a read whose postcheck is still outstanding can be overtaken:
a later read fetches a newer revision, publishes it, and the slow one then
publishes its older revision over the top. Every subsequent call reuses
the older one for the rest of the TTL, so a caller that already saw the
newer content can watch an edit disappear.

Nothing leaves the binding -- both revisions passed complete guard cycles
-- and the overtaking needs the slow read's remaining postcheck and
authorize to outlast the other read's entire cycle, which is more work.
It is still the property `fetches the document once for concurrent reads`
exists to protect, arriving through the sequential path instead of the
concurrent one, so publish only what is at least as recently confirmed as
what is already there.

A guard on publication rather than serializing the whole guarded read,
which would reintroduce the head-of-line blocking the shared fetch exists
to avoid. `>=` so a shared fetch, where both reads hold the same object,
still publishes as a no-op.
The folder picker rendered name plus one of "In a shared drive", the
owner's name, or "My Drive". Two folders called Engineering in different
shared drives are byte-identical rows that mint different capabilities,
and duplicate names across drives are ordinary rather than unlucky. This
lands at the consent step, where the user is deciding what authority to
hand over, and the mistake never surfaces afterwards either: `describe()`
reports the folder's name, so the Connections list shows the same label
for whichever one got bound. The file picker collides the same way when
two files share a name, type and modified date.

Fill the `meta` slot, which is already rendered and already carries
`accessRole` for Calendar and `location`/`type` for BigQuery in this same
file -- the Drive pickers were the ones leaving it empty. A tail of the ID
rather than the whole thing, because `meta` does not shrink and a full ID
would push the subtitle into an ellipsis, trading the useful column for
the disambiguating one; eight characters separate anything inside one page
and still match the tail of the ID in a Drive URL.

This makes a collision visible and checkable, not resolvable: it says the
two rows differ, not which one the user wants. Labelling by containing
drive or parent folder would, and needs either the `/drives` surface this
branch removed or a batched name lookup per search, so it is its own
change.
devin-ai-integration[bot]

This comment was marked as resolved.

Comment thread packages/gatekeeper-google/src/drive-session.ts Outdated
@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown

Posted 1 actionable inline finding.

github run

The folder `getEntry` converted the file and authorized afterwards, so a
conversion failure answered whether an id is a live direct child without
the file being recorded for a later observer to be checked against. Its
account sibling already authorizes the raw id first; this is the same
ordering, and the description uses only `file.id` either way, so nothing
is lost by moving it.

No caller can provoke it. `driveFileToEntry` fails only when Drive omits
`mimeType` or `modifiedTime`, sends a `size` that is not a safe
non-negative integer, or ships a shortcut with no `targetId` -- every one
a violation of the response shape Drive documents and this client
explicitly requests. So this is defensive ordering rather than a closed
leak, which is also why it is worth the two lines: it costs nothing and
removes the one site where the fix needed no judgement.

The paging conversions at the other three call sites are deliberately
untouched. Under the taxonomy those failures are provider outages, not
facts about a file's relationship to the binding, and an outage recorded
as a scope fact is what `isAccountWide` exists to prevent -- so the
expected outcome there is a comment, not a `CursorPager` contract change.
That belongs with the two other findings waiting on the same taxonomy,
and on the prior question of whether a bound path's ancestors are always
covered by recorded folder units, which decides whether a path failure
fences or merely observes. Written up in full outside the repo.
@ask-bonk

ask-bonk Bot commented Sep 18, 2026

Copy link
Copy Markdown

LGTM!

github run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gatekeeper Changes to a gatekeeper integration

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants