Skip to content

docs(sheets): describe Connected Sheets BigQuery authorization accurately - #1023

Merged
steipete merged 3 commits into
openclaw:mainfrom
ryo-touch:docs/connected-sheets-scope-accuracy
Aug 25, 2026
Merged

docs(sheets): describe Connected Sheets BigQuery authorization accurately#1023
steipete merged 3 commits into
openclaw:mainfrom
ryo-touch:docs/connected-sheets-scope-accuracy

Conversation

@ryo-touch

@ryo-touch ryo-touch commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Refs #938.

docs/sheets-connected.md makes two claims about BigQuery authorization that are not true, both shipped in 0.37. This corrects them and nothing else.

What is wrong today

  1. "Google requires https://www.googleapis.com/auth/bigquery.readonly". Google requires BigQuery read access. bigquery.readonly is the least-privilege scope that grants it, and that is why the Connected Sheets client asks for exactly that one — but it is not the only grant Google accepts.
  2. Re-consent is presented as unconditional. An account whose stored grant already covers BigQuery read access needs no further consent round.

The second one matters in practice: a reader with a broadly-scoped token is told to re-authorize, and following that instruction on a token covering many services is a real cost for no gain.

Why (2) is true

The Connected Sheets client performs no local scope check. NewConnectedSheets goes through optionsForAccountScopes, which passes requireStoredGrant=false, so the stored-grant comparison in tokenSourceForAccountScopesWithStoredScopeCheck (internal/googleapi/client_auth.go:437) is bypassed. The stored refresh token is used as-is and the request is authorized by Google against whatever the account already holds, not against the literal scope strings gog names. InsufficientScopeError is only ever constructed inside that skipped branch.

On documenting accepted supersets

You asked for a reproducible redacted account-scope listing plus successful Connected Sheets output for the cloud-platform case. I could not produce evidence that actually isolates the variable, and I would rather say so than attach a listing that looks like proof and is not.

Google accumulates granted scopes. My account's token now holds bigquery.readonly alongside cloud-platform, so any listing from it is consistent with either explanation and proves nothing about cloud-platform on its own. Getting a token with cloud-platform but without bigquery.readonly means revoking the application's authorization and re-consenting from scratch — on a working token covering 22 services — and I have no separate account I can grant narrowly instead.

So this PR deliberately does not enumerate accepted supersets. It states the mechanism (gog does not gate locally; Google decides) and leaves bigquery.readonly as the documented least-privilege default. If you want the superset named explicitly, that needs evidence from an account that can be granted narrowly, and I would rather leave it undocumented than assert it from inference.

This also walks back part of what I claimed in #938: that my read-path validation ran on a cloud-platform token without re-consent. I cannot now establish whether bigquery.readonly was already on that token at the time, so please treat the superset half of that report as unverified. The read-path findings themselves were reproduced and fixed in #1001 and are unaffected.

Scope

Docs only, one file, no command or generated-page changes. make docs-check passes (719 command pages, 27 feature pages).

The lifecycle PRs are unaffected by the above — a throwaway spreadsheet and scratch dataset do not need a separate account — and I am starting with refresh as you asked.

🤖 Generated with Claude Code

…tely

The page presented `bigquery.readonly` as a scope Google requires and
re-consent as unconditional. Both overstate the constraint.

Google requires BigQuery read access; `bigquery.readonly` is the
least-privilege scope that grants it, and that is why the Connected
Sheets client asks for exactly that one. It is not the only grant that
satisfies Google.

The client also performs no local scope check: it builds its token
source through optionsForAccountScopes, so the stored-grant comparison
in tokenSourceForAccountScopesWithStoredScopeCheck is bypassed and the
request is authorized by Google against whatever the account already
holds. An account whose grant already covers BigQuery read access needs
no further consent round, which the previous wording denied.

Keep `bigquery.readonly` as the documented default rather than
enumerating broader scopes, so least-privilege guidance stays intact
while the page stops describing a requirement that is not one.

Refs openclaw#938

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@clawsweeper clawsweeper Bot added P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 25, 2026
@clawsweeper

clawsweeper Bot commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 11:51 AM ET / 15:51 UTC.

ClawSweeper review

What this changes

The PR revises Connected Sheets authorization guidance to cite the required BigQuery scope and require re-authorization only when the stored account lacks the documented grants.

Merge readiness

⚠️ Ready for maintainer review - 1 item remains

This focused docs PR remains necessary: current main still has the unconditional wording, while the PR accurately states the documented explicit scopes and conditional re-authorization path.

Priority: P3
Reviewed head: 1a8e06224dd54c2570fd681295401211329add53

Review scores

Measure Result What it means
Overall readiness 🦞 diamond lobster (5/6) A focused, source-aligned documentation correction with no actionable patch defect found.
Proof confidence 🌊 off-meta tidepool Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Patch quality 🦞 diamond lobster (5/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: Real behavior proof is not required because this PR only changes files under docs/.
Evidence reviewed 4 items Focused current-head diff: The base-to-head comparison changes only two authorization paragraphs: 2 additions and 2 removals.
Client scope contract: Connected Sheets explicitly requests Sheets read-only and BigQuery read-only scopes, separate from ordinary Sheets authorization.
Stored-token path: The normal account-scopes helper disables stored-grant prechecking; the stored scope comparison occurs only when that check is explicitly required.
Findings None None.
Security None None.

How this fits together

Connected Sheets commands create a read-only Sheets API client with Sheets and BigQuery scopes. The documentation guides operators in granting access before the client reads data-source metadata and extracts.

flowchart LR
  A[Stored Google account] --> B[Connected Sheets commands]
  B --> C[Connected Sheets client]
  C --> D[Sheets and BigQuery scopes]
  D --> E[Google Sheets API]
  E --> F[Data source metadata and extracts]
Loading

Before merge

  • Complete next step (P2) - No repair lane is needed; this focused docs-only PR needs ordinary maintainer review once its checks settle.
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Documentation scope 1 file; 2 added, 2 removed The patch is limited to authorization guidance and adds no command, dependency, or generated-document change.

Technical review

Best possible solution:

Merge the narrowly scoped clarification after ordinary maintainer review and completion of the running checks, keeping the upstream guide and explicit two-scope client request as the documented contract.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this docs-only change is verified against the current client’s explicit scope request and stored-token path.

Is this the best way to solve the issue?

Yes. The head preserves the supported exact scopes, links the upstream Connected Sheets guide, and removes the unconditional re-authorization instruction without claiming unverified scope supersets.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 06e86dc46e02.

Labels

Label changes:

  • add rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
  • remove rating: 🐚 platinum hermit: Current PR rating is rating: 🦞 diamond lobster, so this older rating label is no longer current.

Label justifications:

  • P3: This is a low-risk documentation clarification with no runtime behavior change.
  • rating: 🦞 diamond lobster: Overall readiness is 🦞 diamond lobster; proof is 🌊 off-meta tidepool and patch quality is 🦞 diamond lobster.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: Real behavior proof is not required because this PR only changes files under docs/.

Evidence

What I checked:

Likely related people:

  • steipete: Authored the current PR-head revision and the earlier OAuth stored-scope behavior change. (role: recent documentation and authorization-area contributor; confidence: high; commits: 1a8e06224dd5, dbd3846fccda; files: docs/sheets-connected.md, internal/googleapi/client.go, internal/googleapi/client_auth.go)
  • ryo-touch: Authored the merged Connected Sheets read-path work referenced by this documentation PR. (role: merged Connected Sheets feature contributor; confidence: high; commits: c4952a2241c4; files: internal/cmd/sheets_datasource_test.go)

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (2 earlier review cycles)
  • reviewed 2026-08-25T11:56:41.490Z sha 6fd6f50 :: needs maintainer review before merge. :: none
  • reviewed 2026-08-25T12:06:44.122Z sha 7b2eb21 :: needs maintainer review before merge. :: none

ryo-touch and others added 2 commits August 25, 2026 21:03
The note claimed Google authorizes against the existing grant rather
than the literal scopes, without qualification. That holds only for
stored user OAuth tokens.

The service-account path is tried first and asserts the scopes in the
signed JWT via google.JWTConfigFromJSON, so domain-wide delegation does
require the administrator to have approved exactly those strings — which
the preceding paragraph already says. As written the two contradicted
each other.

Narrow the claim to stored OAuth and name the delegation case as the
contrast, so the section reads as one rule with a stated exception.
Also attribute the behavior to the Connected Sheets client rather than
to gog as a whole: other clients do check the stored grant.

Refs openclaw#938

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@clawsweeper clawsweeper Bot added rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. and removed rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. labels Aug 25, 2026
@steipete
steipete merged commit 1251bfc into openclaw:main Aug 25, 2026
8 checks passed
@steipete

Copy link
Copy Markdown
Collaborator

Landed in 1251bfc. Thanks @ryo-touch!

The final wording keeps Google's explicitly documented bigquery.readonly requirement alongside Sheets authorization, makes reauthorization conditional on those permissions not already being present, and preserves the account's existing service selection. I removed the unverified broader-scope equivalence and the overly strict domain-wide-delegation assertion; Google's Connected Sheets guide remains the source of truth: https://developers.google.com/workspace/sheets/api/guides/connected-sheets.

Verification:

  • make docs-check with the existing shared tool directory — generated documentation, site checks, and coverage all passed.
  • go test ./internal/googleapi -run '^TestNewConnectedSheetsRequestsReadOnlySheetsAndBigQueryScopes$' -count=1 — verifies the client requests both read-only Sheets and BigQuery scopes.
  • Independent review passed with no actionable findings.
  • Exact-head hosted CI passed Linux, Windows, macOS/cgo, and the worker: https://github.com/openclaw/gogcli/actions/runs/32867962894.

Issue #938 intentionally remains open for the separately scoped Connected Sheets lifecycle work.

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

Labels

P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. rating: 🦞 diamond lobster Very strong PR readiness with only minor maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants