fix(doc): capture line-wrapped base64 in clipboard data URI regex#586
Merged
fangshuyu-768 merged 1 commit intofeat/media-insert-clipboardfrom Apr 21, 2026
Merged
Conversation
HTML and RTF clipboard content commonly folds base64 payloads at
76 chars (standard MIME folding). The previous character class
[A-Za-z0-9+/\-_]+=* stopped at the first \n, so the downstream
strings.Fields normalisation was a no-op (nothing to strip) and
extractBase64ImageFromClipboard silently uploaded a truncated
payload whose 8-byte prefix happened to pass hasKnownImageMagic.
Extend the class to include \s so the Fields strip actually has
whitespace to remove before base64 decoding. Terminators (", <,
), ;) remain outside the class so the match still ends at the
URI boundary.
Add TestReBase64DataURI_LineWrapped covering \n, \r\n, and \t
folds, full round-trip byte-equality, and the terminator-boundary
invariant so any future regression trips a failing test.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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
domain/ccm
fangshuyu-768
added a commit
that referenced
this pull request
Apr 22, 2026
* feat(doc): add --from-clipboard flag to docs +media-insert
Allow users to upload the current clipboard image directly to a Lark
document without saving to a local file first.
- New --from-clipboard bool flag (mutually exclusive with --file)
- shortcuts/doc/clipboard.go: readClipboardToTempFile() with per-OS impl
macOS — osascript (built-in, no extra deps)
Windows — PowerShell + System.Windows.Forms (built-in)
Linux — tries xclip / wl-paste / xsel in order; clear install hint
on failure
- No new Go dependencies, no Cgo
- Temp file is created before upload and removed via defer cleanup()
- --file changed from Required:true to optional; Validate enforces
exactly-one of --file / --from-clipboard
* fix(doc): fix clipboard image read on macOS for screenshots and browser-copied images
- Add TIFF fallback (macOS screenshots default to TIFF, not PNG)
- Add HTML base64 fallback (images copied from Feishu/browser embed data URI)
- Use current directory for temp file so FileIO path validation passes
* fix(doc): scan HTML/RTF/text clipboard formats for base64 image data URIs
Extend attempt-3 fallback to iterate all text-based clipboard formats
(HTML, RTF, UTF-8, plain text) rather than only HTML. Any format that
contains a "data:<mime>;base64,<data>" pattern is accepted, covering
images copied from Feishu, Chrome, Safari, and other apps that embed
base64 in non-HTML clipboard slots. Also handle URL-safe base64.
* test(doc): add unit tests for clipboard helpers to meet 60% coverage threshold
Cover decodeHex, hexVal, decodeOsascriptData, reBase64DataURI, and
extractBase64ImageFromClipboard (via fake osascript on PATH).
Package coverage: 57% → 61.2%.
* fix(doc): address CodeRabbit review comments on clipboard feature
- Extend reBase64DataURI regex to cover URL-safe base64 chars (-_) so
URL-safe payloads are matched before decoding is attempted
- Fix readClipboardLinux to continue to next tool when a found tool
returns empty output instead of failing immediately
- Guard fake-osascript test with runtime.GOOS == "darwin" skip
- Use os.PathListSeparator instead of hardcoded ":" in test PATH setup
* fix(doc): replace os.* temp-file clipboard path with in-memory streaming
Fixes forbidigo lint violations in shortcuts/doc: os.CreateTemp, os.Remove,
os.Stat, os.WriteFile are banned in shortcuts/; replaced with vfs.* equivalents
for sips TIFF→PNG conversion, and eliminated temp files entirely elsewhere by
having platform clipboard readers return []byte directly.
- readClipboardDarwin: osascript outputs hex literals decoded in Go (no file I/O)
- readClipboardWindows: PowerShell outputs base64 to stdout, decoded in Go
- readClipboardLinux: tool stdout bytes returned directly
- convertTIFFToPNGViaSips: still needs temp files — uses vfs.CreateTemp/Remove
- DriveMediaUploadAllConfig/DriveMediaMultipartUploadConfig: add Content io.Reader
field so in-memory clipboard bytes skip FileIO.Open() path
- Fix ineffassign in clipboard_test.go (scriptBody double-assignment)
- Update TestReadClipboardLinux_NoToolsReturnsError for new signature
* fix(doc): address CodeRabbit review comments on Linux clipboard path
- Update --from-clipboard flag description to list xclip, xsel and wl-paste
- Preserve last backend-specific error in readClipboardLinux so users see
a meaningful message when a tool is found but fails
- Validate PNG magic bytes for xsel output (xsel cannot negotiate MIME types)
- Add URL-safe base64 regression test for reBase64DataURI
* fix(doc): strip whitespace from base64 payload before decoding clipboard data URI
HTML and RTF clipboard content often line-wraps base64 at 76 characters.
FindSubmatch returns the raw wrapped token so direct decode would fail.
Normalize whitespace with strings.Fields before passing to base64.Decode.
* fix(doc): drop TIFF fallback and internal/vfs import on macOS clipboard
depguard rule shortcuts-no-vfs forbids shortcuts/ from importing
internal/vfs directly. The only caller was the sips TIFF→PNG
conversion, which was already a fragile best-effort fallback that
required temp files.
Remove the TIFF fallback entirely; the remaining two attempts cover
the real-world cases:
1. osascript → PNG hex literal — native screenshots and most apps
2. scan text clipboard formats for base64 data URI — Feishu/browsers
* test(doc): cover readClipboardLinux xsel PNG validation and dispatcher path
Added tests:
- TestReadClipboardLinux_XselRejectsNonPNG: fake xsel that returns plain
text is rejected by the PNG-magic check, preventing text from being
uploaded as an "image".
- TestHasPNGMagic: table-driven coverage of the PNG signature check.
- TestReadClipboardImageBytes_UnsupportedPlatform: exercises the shared
dispatcher post-processing and asserts the (nil, nil) invariant.
Raises clipboard.go diff coverage and brings the package from 61.6% to
63.8% overall.
* test: cover in-memory Content upload paths for clipboard feature
Adds unit tests for the new Content io.Reader branches introduced by
the clipboard feature:
- UploadDriveMediaAll with in-memory Content (drive_media_upload.go 87.5%)
- UploadDriveMediaMultipart with in-memory Content (84.6%)
- uploadDocMediaFile single-part and multipart with clipboard bytes
(doc_media_upload.go 0% -> 88.9%)
Adds TestNewRuntimeContextForAPI helper that wires Factory, context,
and bot identity so package tests can invoke DoAPI without mounting
the full cobra command tree.
* test: cover clipboard Validate/DryRun branches and testing helper
Adds unit tests for the clipboard-related Validate/DryRun paths that
Codecov patch-coverage was flagging as uncovered:
- Validate error when neither --file nor --from-clipboard is supplied
- Validate error when both are supplied (mutual exclusion)
- DryRun output contains <clipboard image> placeholder
- Self-test for TestNewRuntimeContextForAPI so shortcuts/common
sees coverage for the new helper (not just shortcuts/doc)
* test: cover Execute clipboard branch via injectable readClipboardImage
Makes readClipboardImageBytes swappable in tests by routing the call
through a package-level variable readClipboardImage. Tests inject a
synthetic PNG payload so the full Execute clipboard flow
(resolve → create block → upload in-memory bytes → bind) runs under
unit test without a real pasteboard.
Covers:
- TestDocMediaInsertExecuteFromClipboard: end-to-end happy path
- TestDocMediaInsertExecuteClipboardReadError: early-return on
readClipboardImage() failure
* ci: re-trigger pull_request workflow for PR #508
Previous push to 9dedb7a did not trigger the main CI workflow via
the pull_request event (only PR Labels ran). The workflow_dispatch
run I triggered manually lacks PR-scoped secrets so security and
e2e-live failed. An empty commit replays the pull_request event so
the full matrix (deadcode, license-header, security, e2e-live) runs
with proper context.
* test(doc): guard info.Size() behind err check to prevent nil-deref
CodeRabbit flagged that 't.Fatalf("... size=%d err=%v", info.Size(), err)'
evaluates info.Size() even when os.Stat returned (nil, err), which nil-derefs.
Split the check into two stages so the error-path t.Fatalf does not touch
info.
* fix(doc): address fangshuyu-768 review on clipboard PR
Seven code changes driven by review feedback:
1. clipboard.go: stop using CombinedOutput() on osascript / powershell.
Stdout is decoded, stderr is captured separately via cmd.Stderr and
surfaced in the terminal error message, so locale warnings or
AppleEvent permission prompts no longer pollute the hex/base64
payload or mask the real failure.
2. clipboard.go: validate decoded base64 data URI bytes against known
image magic headers (PNG/JPEG/GIF/WebP/BMP). A text clipboard that
happens to contain a literal 'data:image/...;base64,...' fragment
(documentation, tutorials, pasted HTML source) no longer silently
becomes an image upload.
3. clipboard.go: simplify the Linux 'no tool found' install hint to a
distro-agnostic phrasing instead of apt/yum only.
4. clipboard_test.go: delete the stale TestReadClipboardToTempFile_*
tests. They referenced a readClipboardToTempFile function that no
longer exists and only exercised os.CreateTemp/os.Remove. Replace
with TestReadClipboardImageBytes_EmptyResultReturnsError which
actually locks in the 'empty clipboard' → error contract of the
current API (Linux-only since mac/Windows need a real pasteboard).
5. doc_media_upload.go: introduce UploadDocMediaFileConfig struct so
uploadDocMediaFile takes a named config instead of 8 positional
params. Drops the //nolint:lll the old call site had to carry.
6. doc_media_insert.go: convert the clipboard upload call to the new
config struct and only set Config.Content when the clipboard branch
actually produced bytes — this also fixes a latent typed-nil bug
where a nil *bytes.Reader was being passed through an io.Reader
parameter, which tripped the 'if cfg.Content != nil' check in
UploadDriveMediaAll and crashed --file uploads.
7. shortcuts/common/testing.go: TestNewRuntimeContextForAPI now takes
the identity as an explicit core.Identity parameter instead of
hardcoding core.AsBot, and its self-test covers both AsBot and
AsUser. Existing call sites pass core.AsBot explicitly.
Also annotates DryRun output with an 'upload_size_note' when
--from-clipboard is set, since DryRun never reads the pasteboard and
can't predict whether the payload will take the single-part or
multipart path.
* fix(doc): capture line-wrapped base64 in clipboard data URI regex (#586)
HTML and RTF clipboard content commonly folds base64 payloads at
76 chars (standard MIME folding). The previous character class
[A-Za-z0-9+/\-_]+=* stopped at the first \n, so the downstream
strings.Fields normalisation was a no-op (nothing to strip) and
extractBase64ImageFromClipboard silently uploaded a truncated
payload whose 8-byte prefix happened to pass hasKnownImageMagic.
Extend the class to include \s so the Fields strip actually has
whitespace to remove before base64 decoding. Terminators (", <,
), ;) remain outside the class so the match still ends at the
URI boundary.
Add TestReBase64DataURI_LineWrapped covering \n, \r\n, and \t
folds, full round-trip byte-equality, and the terminator-boundary
invariant so any future regression trips a failing test.
* docs(skill): add clipboard-empty fallback guidance for +media-insert
When --from-clipboard returns 'no image data' (empty clipboard, non-image
content, or Linux without xclip/wl-paste/xsel), the agent must NOT silently
swallow the error. It should tell the user the clipboard had no image, ask
for a local file path, then retry the same insert command with --file.
Lists three anti-patterns (silent success, guessing a file path, pre-emptive
save-then-file workaround) that agents have been tempted into.
* docs(skill): user-stated source trumps clipboard/file heuristic
The heuristic table (prefer --from-clipboard when image is on the
clipboard) is a fallback for when the user is vague. If the user
explicitly says 'use the screenshot I just copied' → clipboard; if
they give a path → --file. Agent must not silently swap sources even
when the other looks 'better'.
---------
Co-authored-by: fangshuyu-768 <shuyufang768@outlook.com>
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
Targeted fix for PR #508 addressing the review thread at #discussion_r3116012669: the
strings.Fieldsnormalisation added inadac5bfis a no-op, and line-wrapped base64 payloads are silently uploaded as corrupt 8-byte "PNGs".Root cause
reBase64DataURIhas character class[A-Za-z0-9+/\-_]+=*, which excludes whitespace. For a 76-char-folded HTML payload — standard MIME base64 folding, very common in real clipboards (Chrome, Safari, Feishu copy-image-as-HTML) — the regex stops at the first\n, som[2]is already truncated by the time thestrings.Fieldsstrip runs.Fieldsthen normalises a string that by construction has no whitespace, which does nothing.Concretely, a payload like:
matches only
iVBORw0KGgo(11 chars), decodes to exactly the 8-byte PNG signature89 50 4E 47 0D 0A 1A 0A, passeshasKnownImageMagic, and ends up being uploaded as a truncated 8-byte "PNG" block. The user sees a broken image.Fix
Extend the character class to include
\s, so the existingstrings.Fieldsstrip on line 144 ofclipboard.goactually has whitespace to normalise. Terminators (",<,),;) stay outside the class, so the match still ends at the URI boundary.Test plan
go test ./shortcuts/doc/... -count=1passes (newTestReBase64DataURI_LineWrappedadded)go vet ./shortcuts/doc/...cleangofmt -lclean\n,\r\n, and\tthrough the regex, asserts full round-trip byte-equality afterstrings.Fieldsnormalisation, and confirms the regex does not run past the">attribute terminator (catches an over-permissive regression in the other direction).TestReBase64DataURI_Match,TestReBase64DataURI_URLSafeMatch,TestReBase64DataURI_NoMatch,TestExtractBase64ImageFromClipboard_WithFakeOsascript— still pass.Notes
feat/media-insert-clipboardso this lands as part of PR feat(doc): add --from-clipboard flag to docs +media-insert #508 before it merges tomain.🤖 Generated with Claude Code