fix(auth): support comma-separated multi-scope in auth login --scope#764
Open
meijing0114 wants to merge 1 commit intolarksuite:mainfrom
Open
fix(auth): support comma-separated multi-scope in auth login --scope#764meijing0114 wants to merge 1 commit intolarksuite:mainfrom
auth login --scope#764meijing0114 wants to merge 1 commit intolarksuite:mainfrom
Conversation
`lark-cli auth login --scope "a,b"` previously sent the raw comma-joined string to the device authorization endpoint, which treats it as a single malformed scope and fails with: device authorization failed: The provided scope list contains invalid or malformed scopes. OAuth 2.0 (RFC 6749 §3.3) requires space-delimited scopes on the wire, but commas are the more natural separator for users typing on a shell (quoting whitespace is awkward, especially for AI-agent generated commands). Accept both: split on commas/whitespace, trim, dedupe, then re-join with single spaces. Also adds unit tests covering single, comma, space, mixed, dedupe, and trailing-separator inputs.
|
|
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe ChangesScope Normalization in Auth Login
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
github-actions
Bot
added
the
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/auth/login.go`:
- Around line 188-193: The normalized scope (finalScope) can become empty (e.g.,
input " , , ") even though the user passed --scope, so add a validation after
calling normalizeScopeInput(opts.Scope) to detect when opts.Scope is non-empty
but finalScope == "" and return a user-facing validation error; update the login
command handler (around where finalScope is set) to check opts.Scope and
finalScope and fail fast with a clear message like "invalid scope: empty after
normalization" instead of proceeding to the device authorization flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9219c9f-a010-4ea0-b66b-dc83d043338c
📒 Files selected for processing (2)
cmd/auth/login.gocmd/auth/login_test.go
Comment on lines
+188
to
193
| // Normalize --scope so users can pass either OAuth-standard space-separated | ||
| // values or the more natural comma-separated list. RFC 6749 §3.3 mandates | ||
| // space-delimited scopes in the wire request, so the device authorization | ||
| // endpoint rejects raw "a,b" strings as a single malformed scope. | ||
| finalScope := normalizeScopeInput(opts.Scope) | ||
|
|
There was a problem hiding this comment.
Validate normalized scope is not empty when --scope is provided.
At Line 192, normalizeScopeInput can turn inputs like " , , " into "", but the command still proceeds as if scope was provided. Add a guard right after normalization to fail fast with validation error.
Suggested patch
finalScope := normalizeScopeInput(opts.Scope)
+if opts.Scope != "" && finalScope == "" {
+ return output.ErrValidation("please specify at least one valid scope")
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Normalize --scope so users can pass either OAuth-standard space-separated | |
| // values or the more natural comma-separated list. RFC 6749 §3.3 mandates | |
| // space-delimited scopes in the wire request, so the device authorization | |
| // endpoint rejects raw "a,b" strings as a single malformed scope. | |
| finalScope := normalizeScopeInput(opts.Scope) | |
| // Normalize --scope so users can pass either OAuth-standard space-separated | |
| // values or the more natural comma-separated list. RFC 6749 §3.3 mandates | |
| // space-delimited scopes in the wire request, so the device authorization | |
| // endpoint rejects raw "a,b" strings as a single malformed scope. | |
| finalScope := normalizeScopeInput(opts.Scope) | |
| if opts.Scope != "" && finalScope == "" { | |
| return output.ErrValidation("please specify at least one valid scope") | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cmd/auth/login.go` around lines 188 - 193, The normalized scope (finalScope)
can become empty (e.g., input " , , ") even though the user passed --scope, so
add a validation after calling normalizeScopeInput(opts.Scope) to detect when
opts.Scope is non-empty but finalScope == "" and return a user-facing validation
error; update the login command handler (around where finalScope is set) to
check opts.Scope and finalScope and fail fast with a clear message like "invalid
scope: empty after normalization" instead of proceeding to the device
authorization flow.
There was a problem hiding this comment.
@JackZhao10086 I've checked the current code — the concern is still valid. Here's the exact scenario:
- A user runs
lark-cli auth login --scope " , , "(separators only, no real scope tokens). hasAnyOptionevaluates totrue(becauseopts.Scope != ""), so the "no options" interactive prompt/guard is bypassed.normalizeScopeInput(" , , ")returns"".- Since neither
--domainnor--recommendare set,finalScopestays"". - The empty string is forwarded to
RequestDeviceAuthorization, likely producing a cryptic server-side error instead of a clear local validation message.
The suggested guard should be added right after normalization:
finalScope := normalizeScopeInput(opts.Scope)
if opts.Scope != "" && finalScope == "" {
return output.ErrValidation("please specify at least one valid scope")
}This is a minimal, targeted fix that fails fast with a user-friendly message whenever the provided --scope value collapses to nothing after normalization.
🧠 Learnings used
Learnt from: JayYoung2021
Repo: larksuite/cli PR: 410
File: cmd/auth/login_scope_cache.go:47-54
Timestamp: 2026-04-11T23:06:23.250Z
Learning: In larksuite/cli Go code, use `validate.SafeInputPath` only for user-supplied file path inputs (e.g., `--file` flags). Do NOT require `validate.SafeInputPath` for internally constructed, application-managed filesystem paths when: (1) their root directory is already validated via `validate.SafeEnvDirPath()` inside helpers like `appdir.CacheDir()`, `appdir.StateDir()`, or `appdir.LogDir()`, and (2) the filename/path components are sanitized by helpers such as `sanitizeLoginScopeCacheKey()`. In code reviews, missing `SafeInputPath` calls should not be flagged for these app-managed paths if the above safeguards are already in place.
Learnt from: JayYoung2021
Repo: larksuite/cli PR: 410
File: internal/update/update.go:147-147
Timestamp: 2026-04-11T23:06:28.717Z
Learning: In this repo’s Go code (larksuite/cli), treat `validate.SafeInputPath` as a guard for **user-supplied local file paths only**. When you build a path by joining a previously validated base directory (e.g., from `appdir.StateDir()`, `appdir.CacheDir()`, `appdir.ConfigDir()` which already validate env overrides via `validate.SafeEnvDirPath()`) with a compile-time constant filename (e.g., `"update-state.json"`, `"config.json"`), do **not** require an additional `validate.SafeInputPath` call before using `vfs.*` or `validate.AtomicWrite`. Reviewers should avoid flagging/adding redundant `validate.SafeInputPath` in these computed-path cases; only require it when the path ultimately comes from user input.
Learnt from: ViperCai
Repo: larksuite/cli PR: 450
File: shortcuts/slides/slides_create.go:50-60
Timestamp: 2026-04-14T03:13:09.459Z
Learning: When reviewing this Go codebase, do not require explicit `validate.SafeInputPath` / `validate.SafeOutputPath` calls for user-supplied paths if the code uses the `runtime.FileIO()` abstraction methods (`Stat`, `Open`, `Save`, `ResolvePath`). `runtime.FileIO()` is backed by `LocalFileIO` (in `localfileio.go`), which enforces `SafeInputPath`/`SafeOutputPath` before any filesystem access and reports violations as `fileio.PathValidationError`. Only require explicit `validate.SafeInputPath` when bypassing `runtime.FileIO()` and calling `vfs.*` or `os.*` directly with user-influenced paths.
meijing0114
force-pushed
the
fix/auth-login-multi-scope
branch
from
eb1ab99 to
9f20c8a
Compare
Collaborator
|
hi，meijing0114 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #764 +/- ##
==========================================
+ Coverage 65.40% 65.43% +0.03%
==========================================
Files 508 509 +1
Lines 46795 47035 +240
==========================================
+ Hits 30605 30779 +174
- Misses 13548 13592 +44
- Partials 2642 2664 +22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@9f20c8aeabfd0113b6416a9823f54a2989aaebf6🧩 Skill updatenpx skills add meijing0114/cli#fix/auth-login-multi-scope -y -g |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug repro
lark-cli auth login --scope "vc:note:read,vc:meeting.meetingevent:read"fails with:
A single scope (
--scope "vc:note:read") works, so the comma-joined value is the trigger.Root cause
cmd/auth/login.goreads--scopeas a rawStringVarand passes it through unchanged toRequestDeviceAuthorization. OAuth 2.0 (RFC 6749 §3.3) requires thescopeparameter to be a space-delimited list on the wire, so a comma-joined string is treated as one malformed scope by the auth endpoint.The flag help advertises "space-separated" but most users (and AI agents synthesizing commands) reach for commas first because shell-quoting whitespace is awkward — and
--domainalready accepts,(--domain calendar,task), so the inconsistency is an easy footgun.Fix
Normalize
--scopeinput before sending: split on commas/whitespace, trim, dedupe (first occurrence wins, order preserved), re-join with single spaces. Implementation is a pure helpernormalizeScopeInputnext to the other small helpers inlogin.go— no behavior change for already-valid space-separated inputs.Updated flag help string from
space-separatedtospace- or comma-separated.Test cases
Added
TestNormalizeScopeInputcovering:Existing 87 tests in
cmd/auth/...all still pass.User scenario
I keep adding new VC API endpoints to my agent workflows, and every new endpoint means another
auth loginto grant the scope. Composing the command via shell or AI agent,--scope "a,b"is the natural reach — with this fix it just works, instead of returning a confusing "malformed scopes" error that requires reading docs to learn the OAuth wire format.Summary by CodeRabbit
New Features
auth logincommand's--scopeflag now accepts comma-, space-, or mixed-separated inputs; inputs are trimmed, empties removed, duplicates deduplicated (preserving first-seen order), and normalized to a single space-delimited scope string for device authorization.Tests