feat(auth): add auth qrcode subcommand and update auth docs/hints#968
Open
JackZhao10086 wants to merge 6 commits into
Open
feat(auth): add auth qrcode subcommand and update auth docs/hints#968JackZhao10086 wants to merge 6 commits into
JackZhao10086 wants to merge 6 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds ChangesAuth QR Code Generation Feature
Sequence Diagram(s)sequenceDiagram
participant User as "User/Agent"
participant CLI as "lark-cli auth qrcode"
participant Runner as "runQRCode"
participant Encoder as "go-qrcode / qrcode.New"
participant Storage as "vfs.WriteFile / stdout"
User->>CLI: invoke with URL, --output/--ascii, --size
CLI->>Runner: populate QRCodeOptions and execute
Runner->>Encoder: encode URL to PNG or ASCII
Encoder->>Storage: write PNG to file or print ASCII to stdout
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
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)
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/qrcode.go`:
- Around line 85-87: Replace the direct os.WriteFile call with the repository
filesystem abstraction and validate the CLI-provided path first: call
validate.SafeInputPath(outputPath) (returning the appropriate output.Errorf on
failure) and then use vfs.WriteFile(ctxOrFS, outputPath, png, 0644) (or the
package's equivalent vfs.WriteFileFileSystem method) instead of os.WriteFile;
keep the same error handling that returns output.Errorf(output.ExitInternal,
"write_error", fmt.Sprintf("failed to write QR code to %s: %v", outputPath,
err)) when the vfs write fails.
🪄 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: 784fa3d7-b7db-4469-93ee-43a714434e0f
📒 Files selected for processing (6)
cmd/auth/auth.gocmd/auth/login.gocmd/auth/login_messages.gocmd/auth/login_test.gocmd/auth/qrcode.goskills/lark-shared/SKILL.md
Comment on lines
+85
to
+87
| err = os.WriteFile(outputPath, png, 0644) | ||
| if err != nil { | ||
| return output.Errorf(output.ExitInternal, "write_error", fmt.Sprintf("failed to write QR code to %s: %v", outputPath, err)) |
There was a problem hiding this comment.
Replace direct os.WriteFile with repo-safe filesystem/path handling.
This write path uses a user-supplied CLI argument directly with os.WriteFile, which bypasses the repository’s filesystem abstraction and path-safety expectations.
Proposed fix
import (
"context"
"fmt"
- "os"
"github.com/skip2/go-qrcode"
"github.com/spf13/cobra"
"github.com/larksuite/cli/internal/cmdutil"
"github.com/larksuite/cli/internal/output"
+ "github.com/larksuite/cli/internal/vfs"
)
@@
- err = os.WriteFile(outputPath, png, 0644)
+ // Validate outputPath with the repo-standard path validator for untrusted CLI paths.
+ err = vfs.WriteFile(outputPath, png, 0644)
if err != nil {
return output.Errorf(output.ExitInternal, "write_error", fmt.Sprintf("failed to write QR code to %s: %v", outputPath, err))
}As per coding guidelines: "**/*.go: Use vfs.* instead of os.* for all filesystem access to enable test mocking" and "Validate paths before reading with validate.SafeInputPath because CLI arguments are untrusted."
🤖 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/qrcode.go` around lines 85 - 87, Replace the direct os.WriteFile
call with the repository filesystem abstraction and validate the CLI-provided
path first: call validate.SafeInputPath(outputPath) (returning the appropriate
output.Errorf on failure) and then use vfs.WriteFile(ctxOrFS, outputPath, png,
0644) (or the package's equivalent vfs.WriteFileFileSystem method) instead of
os.WriteFile; keep the same error handling that returns
output.Errorf(output.ExitInternal, "write_error", fmt.Sprintf("failed to write
QR code to %s: %v", outputPath, err)) when the vfs write fails.
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@6d6733a11330f963d417e64a790119f3faca9241🧩 Skill updatenpx skills add larksuite/cli#feat/auth_qrcode -y -g |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #968 +/- ##
==========================================
+ Coverage 67.39% 67.42% +0.02%
==========================================
Files 572 573 +1
Lines 53659 53705 +46
==========================================
+ Hits 36165 36209 +44
- Misses 14486 14487 +1
- Partials 3008 3009 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
JackZhao10086
force-pushed
the
feat/auth_qrcode
branch
from
6513023 to
6d6733a
Compare
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
1 participant
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.
Summary
Add
lark-cli auth qrcodesubcommand and update all auth login hints to require AI agents to generate QR codes for verification URLs, improving the user experience during OAuth device flow authorization.Changes
lark-cli auth qrcode <url>subcommand that generates QR codes in two formats: PNG (--outputfor file path) and ASCII (--asciifor terminal output)AgentTimeoutHintin both Chinese and English to require QR code generation with clear decision rules: prefer PNG, use ASCII only in pure terminal/CLI environments--no-wait --jsonhint in login.go with QR code and display order guidancebind_messages.goMessageUserDefaultwith QR code requirementslark-shared/SKILL.mdURL forwarding rules with QR code generation requirementsauth login -hLong description to mention QR code supportTest Plan
go test ./cmd/auth/...)lark-cli auth qrcode <url> --asciioutputs ASCII QR code to stdoutlark-cli auth qrcode <url> --output /tmp/qrcode.pnggenerates PNG filelark-cli auth qrcodewithout URL argument returns errorlark-cli auth qrcode -hshows correct usage with positional<url>argumentRelated Issues
Summary by CodeRabbit
New Features
Documentation
User-facing messages
Tests