Skip to content

fix(google): shared-drive params on Drive v3 proxy (Bug 2)#251

Open
chitcommit wants to merge 2 commits into
mainfrom
fix/gdrive-shared-drive-params
Open

fix(google): shared-drive params on Drive v3 proxy (Bug 2)#251
chitcommit wants to merge 2 commits into
mainfrom
fix/gdrive-shared-drive-params

Conversation

@chitcommit

@chitcommit chitcommit commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Bug 2 — Drive proxy omits shared-drive params

The Google Drive proxy (src/api/routes/google.js) never set supportsAllDrives=true on any Drive v3 get/export/download call, nor the shared-drive corpora params on list. Files living in a shared drive therefore 404 even with a valid delegated token whose impersonated user is a member of that drive.

Changes

  • GET /gdrive/files (list): supportsAllDrives=true, includeItemsFromAllDrives=true, corpora=allDrives
  • GET /gdrive/files/:fileId (metadata): supportsAllDrives=true
  • GET /gdrive/files/:fileId/content: supportsAllDrives=true on the metadata pre-fetch, the Google-native export URL, and the alt=media download URL

The content route fetches metadata first, so the metadata 404 is what broke the entire download flow.

Verification (ground truth)

File 1pfPB3gq_l2LUWW29k3hadCi0q5F507-i — a 1.84 MB PDF in the "Arias V Bianchi" shared drive (driveId 0AHH7mqSeNtbbUk9PVA). Fetched directly against the Drive v3 API with a delegated service-account token (sub=nick@jeanarlene.com, scope drive), mirroring the prod rotation path exactly:

Call Result
files.get?fields=mimeType (no param) 404 File not found
files.get?fields=mimeType&supportsAllDrives=true 200
files.get?alt=media&supportsAllDrives=true (content) 200 + 1,844,037 bytes
files.list with shared-drive corpora params 200 (lists shared-drive contents)

GAM fetches the same file as ground truth, confirming the delegated subject is a member of the shared drive.

Deploy gate

Do NOT auto-merge. Operator approves prod deploys of new code. This PR is the code half of the shared-drive fix; the credential half (Bug 1, augmented SA JSON in CREDENTIAL_CACHE KV) has already been provisioned out-of-band.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added support for accessing and listing files from Google shared drives across file retrieval and download operations.
  • Tests

    • Enhanced test coverage to verify shared-drive functionality works correctly for file listing, metadata retrieval, and content downloads.

The Google Drive proxy omitted `supportsAllDrives` on all file get/export/
download calls and the shared-drive corpora params on list, so files living
in a shared drive 404'd even with a valid delegated token whose impersonated
user is a member of that drive.

Changes (src/api/routes/google.js):
- GET /gdrive/files (list): add supportsAllDrives=true,
  includeItemsFromAllDrives=true, corpora=allDrives
- GET /gdrive/files/:fileId (metadata): add supportsAllDrives=true
- GET /gdrive/files/:fileId/content: add supportsAllDrives=true to the
  metadata pre-fetch, the Google-native export URL, and the alt=media
  download URL

Proven against ground-truth file 1pfPB3gq_l2LUWW29k3hadCi0q5F507-i (1.84MB
PDF in the "Arias V Bianchi" shared drive, driveId 0AHH7mqSeNtbbUk9PVA),
fetched with a delegated SA token (sub=nick@jeanarlene.com, drive scope):

  files.get?fields=mimeType                        -> 404 File not found
  files.get?fields=mimeType&supportsAllDrives=true -> 200
  files.list (shared corpora params)               -> 200 (lists drive contents)

The content route fetches metadata FIRST, so the metadata 404 is what broke
the whole download flow.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 14, 2026 11:27
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0c029d33-136d-4ac0-8b3b-76c65dfa06db

📥 Commits

Reviewing files that changed from the base of the PR and between c27eab2 and 1e42577.

📒 Files selected for processing (2)
  • src/api/routes/google.js
  • tests/api/google-routes.test.js

📝 Walkthrough

Walkthrough

Three Google Drive proxy route handlers (/gdrive/files, /gdrive/files/:fileId, and /gdrive/files/:fileId/content) are updated to include supportsAllDrives=true and related shared-drive parameters in all upstream Drive API calls. Corresponding tests are added to assert these parameters appear on every affected request URL.

Changes

Google Drive Shared-Drive Support

Layer / File(s) Summary
Shared-drive parameters added to Drive proxy routes
src/api/routes/google.js
/gdrive/files appends supportsAllDrives=true, includeItemsFromAllDrives=true, and corpora=allDrives; /gdrive/files/:fileId adds supportsAllDrives=true to the metadata request; /gdrive/files/:fileId/content adds supportsAllDrives=true to both the metadata prefetch and the constructed export/alt=media download URLs.
Tests verifying shared-drive parameters
tests/api/google-routes.test.js
New assertions confirm all three shared-drive parameters on the files.list URL, supportsAllDrives=true on files.get, and supportsAllDrives=true on both the metadata prefetch and follow-up export/download URLs for the content route.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • chittyos/chittyconnect#160: Introduced the original Google Drive proxy routes in src/api/routes/google.js that this PR extends with shared-drive parameter support.

Poem

🐇 Hopping through the Drive with glee,
supportsAllDrives=true sets shared folders free!
Corpora wide and items all included,
No shared drive left excluded.
The warren of files, open to me! 🗂️

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Description check ❓ Inconclusive The description covers the core problem, changes made, verification methodology, and deploy gate; however, it deviates significantly from the template structure (missing Summary/Security/Docs/Validation sections with checkboxes). Restructure the description to match the template: add Summary, Security & Access, Docs, and Validation sections with appropriate checkboxes and confirmations.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding shared-drive parameters to the Google Drive v3 proxy to fix a bug where files in shared drives returned 404 errors.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/gdrive-shared-drive-params

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
❌ Deployment failed
View logs
chittyconnect 1e42577 Jun 18 2026, 07:38 AM

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes Google Drive v3 proxy behavior for files stored in shared drives by ensuring the required shared-drive query parameters are included on list/get/download flows.

Changes:

  • Add shared-drive list parameters (supportsAllDrives, includeItemsFromAllDrives, corpora=allDrives) to GET /gdrive/files.
  • Add supportsAllDrives=true to GET /gdrive/files/:fileId metadata requests.
  • Add supportsAllDrives=true to the content route’s metadata prefetch and to both export and alt=media download URLs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/api/routes/google.js Outdated
Comment on lines +97 to +98
// Shared-drive support: without these, files living in shared drives are
// invisible to list/get even when the delegated user is a member.
Comment thread src/api/routes/google.js
Comment on lines +99 to +101
params.set("supportsAllDrives", "true");
params.set("includeItemsFromAllDrives", "true");
params.set("corpora", "allDrives");
Comment thread src/api/routes/google.js
Comment on lines +118 to +119
// Shared-drive support — required for files that live in a shared drive.
params.set("supportsAllDrives", "true");
Comment thread src/api/routes/google.js
Comment on lines +140 to 142
const metadataResponse = await fetch(`${DRIVE_API}/files/${encodedFileId}?fields=mimeType&supportsAllDrives=true`, {
headers: { Authorization: `Bearer ${token}` },
});
@chitcommit

Copy link
Copy Markdown
Contributor Author

@claude Please remediate the open review findings on this PR (fix/gdrive-shared-drive-params) so it becomes merge-ready. All are in src/api/routes/google.js and its tests:

  1. ~line 98: the comment claims the shared-drive params affect both list and get, but supportsAllDrives/includeItemsFromAllDrives/corpora only apply to files.list (files.get uses supportsAllDrives separately). Fix the comment to be accurate.
  2. ~line 101: the forced shared-drive list params are a behavior change with no test. Add an assertion in tests/api/google-routes.test.js that the files.list upstream URL includes the shared-drive params.
  3. ~line 119: files.get now depends on supportsAllDrives=true — add a test asserting the upstream files.get URL includes supportsAllDrives=true.
  4. ~line 142: add assertions that supportsAllDrives=true is on the metadata prefetch AND the downstream export/alt=media download URL.

Constraints: tests must exercise real behavior (no new DB/service mocks per repo rules). Run npm test -- google and npx eslint src/api/routes/google.js — all must pass. Commit and push to this branch. Reply to and resolve each addressed review thread. Do NOT merge and do NOT apply governance labels.

@claude

claude Bot commented Jun 18, 2026

Copy link
Copy Markdown

Claude finished @chitcommit's task in 4m 29s —— View job


Review findings remediated

All four Copilot review findings have been addressed in commit 1e42577.

  • Comment fix (line 97–98) — Replaced the misleading "invisible to list/get" comment with a precise one that names which params are list-only (includeItemsFromAllDrives, corpora) and which also apply to get/download routes (supportsAllDrives).
  • Test: files.list shared-drive params — Added "always sets shared-drive params on the upstream files.list URL" asserting all three params (supportsAllDrives=true, includeItemsFromAllDrives=true, corpora=allDrives) are present on every upstream list call.
  • Test: files.get supportsAllDrives — Added "includes supportsAllDrives=true on the upstream files.get URL" in the GET /gdrive/files/:fileId describe block.
  • Tests: content route metadata + export/alt=media — Added two tests in the GET /gdrive/files/:fileId/content describe block:
    • "includes supportsAllDrives=true on the metadata prefetch and the export URL for Google-native files" (checks both the metadata pre-fetch and the /export download URL)
    • "includes supportsAllDrives=true on the metadata prefetch and the download URL" (checks both the metadata pre-fetch and the alt=media download URL)
  • Committed and pushed to fix/gdrive-shared-drive-params
  • ⚠️ Review thread resolutiongh api and npm commands require user approval in this CI environment. CI will validate npm test -- google and npx eslint src/api/routes/google.js on push. Please resolve the four Copilot threads manually once CI is green (the code/test fix for each is in this commit).

What changed

File Change
src/api/routes/google.js Replaced 2-line misleading comment with 4-line accurate one at lines 97–100
tests/api/google-routes.test.js +43 lines: 4 new it() blocks covering all shared-drive param assertions
· Branch

Addresses all four review findings from Copilot:

1. Clarifies the files.list comment — notes that includeItemsFromAllDrives
   and corpora are list-only; supportsAllDrives is set separately on the
   files.get and content routes.
2. Asserts supportsAllDrives, includeItemsFromAllDrives, and corpora are
   always present on the upstream files.list URL.
3. Asserts supportsAllDrives=true is present on the upstream files.get URL.
4. Asserts supportsAllDrives=true is present on the metadata prefetch and
   on both the export and alt=media download URLs in the content route.

Co-authored-by: @chitcommit <chitcommit@users.noreply.github.com>
@chitcommit chitcommit added security-approved Security review approved docs-approved Documentation review approved access-reviewed Access control review approved labels Jun 18, 2026
@chitcommit
chitcommit enabled auto-merge (squash) June 18, 2026 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

access-reviewed Access control review approved docs-approved Documentation review approved security-approved Security review approved

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants