Add a unified Google Drive Folder resource by ndisidore · Pull Request #440 · cloudflare/cloudflare-os

ndisidore · 2026-09-03T21:08:16Z

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.

github-actions · 2026-09-03T22:58:51Z

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

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

Dashboard · deleted when this PR closes

ask-bonk · 2026-09-16T17:54:01Z

Posted 1 actionable inline finding.

github run

ask-bonk · 2026-09-16T20:51:20Z

Posted 2 actionable inline findings.

github run

ndisidore · 2026-09-17T12:35:02Z

@ask-bonk review this

ask-bonk · 2026-09-17T12:53:04Z

Posted 1 actionable inline finding.

github run

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.

ask-bonk · 2026-09-18T14:56:18Z

Posted 3 actionable inline findings.

github run

ask-bonk · 2026-09-18T15:16:07Z

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.

ask-bonk · 2026-09-18T16:22:01Z

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 · 2026-09-18T21:01:00Z

LGTM!

github run

Converging the shared drive and folder resources replaced the `.../drive/folders/:driveId` urlPattern with `.../drive/folders/:folderId`. The two match the same URLs, so bound resource URLs keep parsing and reads keep working, but `urlPattern` is exact-string identity, so an account that recorded the old pattern now holds a grant naming a resource that no longer exists. `recordedResourceUrlPatterns()` returned that string verbatim, and both `reconnect()` and `ensureResources()` feed it straight back into `prepareOAuthFlow()`, where `validateResourceUrlPatterns()` throws "Unknown grantable resource URL pattern(s)". Worse, `grantedResourceUrlPatterns()` already dropped it silently, so the resource read as ungranted, which is exactly what makes `ensureResources()` decide a prompt is needed -- the path that repairs the account was the path that detonated. Drop a retired pattern at the single source instead. It maps to no scopes, so it can only throw; the account re-consents under the live pattern, and because both resources always required the same three narrow scopes, Google returns immediately without a new consent screen. Still outstanding, and not fixed here: an admin who disabled the old shared-drive resource has that urlPattern in AdminConfig's disable set, which now matches nothing, so the folder resource is offered again. Fixing that means migrating admin config in workshop-backend.

ask-bonk · 2026-09-21T22:53:19Z

Medium packages/gatekeeper-google/src/drive-session.ts:742: Direct-child non-folders are rejected without recording a file observation, allowing future collaborators to inherit the probe result unchecked. Authorize the file before rejecting, as openFolder() does.
Low packages/gatekeeper-google/src/google.ts:3065: @validateRpc() still validates DriveSearchQuery, so the new childFolderIds field is forwarded unvalidated. Malformed values reach id.trim() instead of being rejected at the RPC boundary. Include the field in the concrete validated signature and add boundary coverage.

github run

Polling sibling folders cost a request each: `openFolder` per child, then `list`/`search` on each returned capability. For a "Google Meet" folder with a subfolder per meeting, that is a fan-out that grows with meeting history, and it is the shape any "watch this tree" workflow takes. `search()` now accepts `childFolderIds`, searching inside named direct child folders instead of the positioned one. The proof is unchanged, not relaxed: every named folder must still be a listable direct child, established by one batched `getScopeNodes` call and revalidated on every page, and each returned file must sit directly under one of the proven parents or the page is refused. Navigation still does the proving, so this removes repeated requests rather than widening what a folder binding can reach: no recursion, no ancestry walk, no new scope. Measured cost for one page over two named folders is four proof batches plus one `files.list`, against roughly six requests per folder before. One class serves both Drive session interfaces, so the RPC method now declares `DriveSessionSearchQuery` -- the widest shape either accepts. capnweb-validate forwards properties absent from the declared type without checking them, so while the field reached the core either way, a malformed one surfaced as `id.trim is not a function` from inside the query builder rather than as an RPC rejection. The account core refuses `childFolderIds` through the same shape of guard `directParentId` already had: ignoring it would search the whole account while the caller believed the read was narrowed. `#refuseUnlistable` is now shared by `openFolder` and the batched child-folder proof. A named child that is a direct child but not a listable folder is an objective refusal, so it is recorded as a file unit; only `canListChildren` is owner-relative and fences. Writing that branch a third time by hand is what left the batched proof recording nothing at all. A page proves its scope once before the fetch and once after, not three times: `buildEntries` and the authorizer run back to back with no provider read between them, so they share one proof while the pre-fetch check stays separate. That straddle is the actual guarantee and was not collapsed. The set is capped at `MAX_QUERY_PARENTS` (50). Drive documents no `q` length limit, and the batched parent proof chunks at 100, so an uncapped set would pass the proof and then fail the search with an opaque provider 400. The cap is enforced at the query builder as the invariant and again when the query is normalized, so a caller is refused synchronously rather than from the first page of a cursor it already holds. Every named folder is recorded as an observation, so one that later stops being listable fails collaborator admission for the whole set, where opening folders individually keeps each folder's disclosure independent. That coupling is the part most worth a second opinion. `DriveFileQuery.directParentId` becomes `directParentIds`, since the provider boundary is where one-or-many belongs. An empty set throws at both layers rather than being dropped -- dropping a narrowing widens the read to the whole binding, which is the opposite of what was asked. `isDirectChild` now takes a `DriveScopeNode` as its child, so the batched node-vs-node check reuses the one definition instead of a parallel copy; `DriveFile` satisfies that shape, so existing callers are unchanged. `list()` deliberately does not take `childFolderIds`. Its empty result is certified by the folder units the call already records, so it needs no fence -- but a second entry point into a shape with an open question about admission coupling is not worth adding before that question is answered.

Every owner-only read staged a fresh UUID marker before asking for approval, so an activation that died mid-read left admission closed rather than open. Once `OBSERVER_WITHHELD_KEY` is set that fence is permanent, and the marker protects nothing: `#observationWithheld()` returns on the latch before it scans the prefix, so a further marker cannot change its answer. What it can do is cost three storage writes per read and strand a key that will never be read again. In a folder binding `authorizeWithheld` fires on invisible probes, unlistable-folder checks and every empty search, so this is a warm path rather than a rarity, and the stranded keys accumulate without bound -- unlike `gatekeeper-kit`, whose tracker sweeps them in `#compactWithholds`. Return the current exclusions with a no-op commit and no marker instead. A marker stranded *before* the latch is still deliberate: that is what keeps admission closed over a record the overseer may already hold. The divergence this exposed -- that this tracker has no equivalent of the kit's compaction -- is left alone; whether this package should be on the kit's tracker at all is a refactor, not this fix.

ask-bonk · 2026-09-22T00:02:21Z

Posted 1 actionable inline finding.

github run

Three layers decide what a pasted or agent-supplied resource URL becomes, and the middle one disagreed with its neighbours. The host chooses to prefill with `matchesResourceUrlPattern`, which tries both slash forms because, as its own doc comment says, humans and LLMs routinely produce a trailing one. The server mints the capability from `parseResourceUrl`, whose folder arm ends `\/?$` and accepts both. Between them, `defaultValuesFromResourceUrl` ran a single strict `URLPattern.exec`, so `.../drive/folders/FOLDER123/` landed the caller on the right configurator with an empty field and no error. This branch is what makes that reachable rather than cosmetic: with explicit shared-drive enumeration and the source selector gone, a concrete folder URL is the only route to a folder the single provider page misses -- which the README advertises -- and the picker deliberately has no manual URL field, so a failed seed leaves nothing to type into. Fixed in the shared fallback rather than as an `initialValuesFromResourceUrl` hook on the Drive folder module. Every configurator without that hook had the same gap, and the module is transpiled on its own and cannot import the parser, so a local fix would have been a third copy of the URL grammar beside `resources.ts` and the `resourceUrl` encoder. Trying the slash-toggled form is monotone -- it can only turn a non-match into a match -- and it makes extraction agree with what will actually be minted, so the prefilled form cannot preview a different capability from the one the URL names. The strip is `/\\/+$/` in the builder, not `/\/+$/`: this runs inside the runtime template literal, where a single backslash collapses and emits `//`, commenting out the rest of the line and leaving every generated configurator unparseable. The doubled-slash case in the new test exists to fail if that escaping regresses. Coverage goes through `readRuntimeFunctions`, which evaluates the function out of the built runtime. `configurator-url.test.ts` carries a hand copy of this fallback, so a round-trip test there would have passed throughout and would not prove the fix.

ask-bonk · 2026-09-22T12:23:32Z

Posted 2 actionable inline findings.

github run

* origin/main: Fix getting stuck on a failed MCP approval (cloudflare#566) Structured action fields (cloudflare#565) Fix getting stuck verifying access to a shared workspace (cloudflare#559) Upgrade pi to 0.87.1 for Claude 5.5 and GPT-6 (cloudflare#556) Assert XLSX batching by compressor writes, not wall-clock time (cloudflare#562) Complete action descriptions for public gatekeepers (cloudflare#541) Cap tool results at 32K; add grep and line ranges to readFile (cloudflare#494) Fix revert elision for a readFile in the same step as the reverted edit (cloudflare#522) feat(gatekeeper-kit): oauth 2.0 token client (cloudflare#555) Review with GPT-6 Sol and opencode 1.18.32 in Bonk (cloudflare#550) Add a unified Google Drive Folder resource (cloudflare#440) Convert the remaining .mjs build scripts to TypeScript (cloudflare#549) Reconnect the agent session; run trials concurrently; render trajectories (cloudflare#512) Add shared UI package and hierarchical list (cloudflare#484) Owner-invites-only latch for workspaces that observed opted-in sensitive data (cloudflare#523) Frontend: Invite several people at once from the share modal (cloudflare#526) Extend code editor UI to cover worktrees (cloudflare#513) Document and test staged gatekeeper reconnects (cloudflare#520) Fix bundled blueprint test PID assumption (cloudflare#524) Default evals to GPT 5.6 Luna with ten trials per task (cloudflare#521)

Merge of foundation/main tip `a43210a7` into the fork. Upstream range: structured action fields (cloudflare#565, cloudflare#541), pi 0.87.1 (cloudflare#556), grep tool + 32K caps + readFile windows (cloudflare#494), revert elision (cloudflare#522), failed-approval discard (cloudflare#566), OAuth token client (cloudflare#555), Drive folder resource (cloudflare#440), .mjs-to-.ts (cloudflare#549), shared-workspace verify fix (cloudflare#559), XLSX test (cloudflare#562), Bonk review config (cloudflare#550). Hand resolutions (Tier 2, upstream-first) and custom-code reductions (maxArguments retired, GLM override removed, blindfold consolidated into scanWorkpieceForGrep) are detailed in the merge commit message. Fork gates pass: fork-boundary, sync-upstream, upstream-merge-audit (82/82). Reviewed by two independent agents; fix pass clean.

github-actions Bot added the gatekeeper Changes to a gatekeeper integration label Sep 3, 2026