Skip to content

docs(im): clarify media path restrictions#956

Open
mtsui-cmyk wants to merge 1 commit into
larksuite:mainfrom
mtsui-cmyk:docs/im-media-path-help
Open

docs(im): clarify media path restrictions#956
mtsui-cmyk wants to merge 1 commit into
larksuite:mainfrom
mtsui-cmyk:docs/im-media-path-help

Conversation

@mtsui-cmyk
Copy link
Copy Markdown
Contributor

@mtsui-cmyk mtsui-cmyk commented May 19, 2026

Summary

Clarify the IM media flag contract for local uploads: existing keys and URLs are accepted, while local paths must be cwd-relative and stay inside the current directory. This addresses the help-text gap reported in #872.

Changes

  • Share IM media flag descriptions across +messages-send and +messages-reply so --help documents URL support and the relative-path restriction.
  • Update lark-im send/reply skill references with media input rules and absolute-path workarounds.
  • Add unit coverage that the media flag descriptions mention URLs and path restrictions.

Test Plan

  • Unit tests pass
  • Manual local verification confirms the lark xxx command works as expected

Commands run:

  • PATH="$PWD/.tmp/go-toolchain/go/bin:$PATH" go test ./shortcuts/im
  • PATH="$PWD/.tmp/go-toolchain/go/bin:$PATH" go test -race -count=1 ./shortcuts/im
  • .tmp/go-toolchain/go/bin/gofmt -l shortcuts/im/helpers.go shortcuts/im/im_messages_send.go shortcuts/im/im_messages_reply.go shortcuts/im/validate_media_test.go
  • git diff --check
  • PATH="$PWD/.tmp/go-toolchain/go/bin:$PATH" go run . im +messages-send --help
  • PATH="$PWD/.tmp/go-toolchain/go/bin:$PATH" go run . im +messages-reply --help

Related Issues

Summary by CodeRabbit

  • Documentation

    • Clarified media input behavior for IM commands—now explicitly supports resource keys, remote URLs, and cwd-relative local paths
    • Added Media Input Rules sections detailing security constraints: local paths must be cwd-relative, absolute paths rejected, and parent directory traversal (..) blocked
  • Refactor

    • Consolidated media flag descriptions into shared constants for consistency across commands

Review Change Stack

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 19, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact labels May 19, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 19, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a9f7757f-21f4-4a90-a2cf-b66b18721ee5

📥 Commits

Reviewing files that changed from the base of the PR and between 315e0ab and 759eb62.

📒 Files selected for processing (6)
  • shortcuts/im/helpers.go
  • shortcuts/im/im_messages_reply.go
  • shortcuts/im/im_messages_send.go
  • shortcuts/im/validate_media_test.go
  • skills/lark-im/references/lark-im-messages-reply.md
  • skills/lark-im/references/lark-im-messages-send.md

📝 Walkthrough

Walkthrough

This PR documents undocumented path restrictions on IM media flags by introducing shared flag description constants, synchronizing both command implementations to use them, updating skill reference documentation to explicitly describe supported input types (keys, URLs, cwd-relative paths), and adding tests to enforce consistent documentation of path safety rules.

Changes

IM Media Path Restrictions Documentation

Layer / File(s) Summary
Shared flag description constants and synchronization
shortcuts/im/helpers.go, shortcuts/im/im_messages_reply.go, shortcuts/im/im_messages_send.go
Defines centralized flag description strings documenting that media flags accept existing keys, remote URLs, and cwd-relative local paths with path safety validation. Both ImMessagesSend and ImMessagesReply reference these shared descriptions instead of hardcoded strings.
Test validation for flag descriptions
shortcuts/im/validate_media_test.go
Adds test function and helper to verify that media flag descriptions document cwd-relative path requirements, URL handling, and rejection of absolute paths and relative traversal.
Messages-reply skill documentation
skills/lark-im/references/lark-im-messages-reply.md
Updates documentation with explicit support for keys and URLs, introduces Media Input Rules section specifying cwd-relative path validation, and broadens parameter descriptions to document path|url|key inputs.
Messages-send skill documentation
skills/lark-im/references/lark-im-messages-send.md
Updates documentation with explicit support for keys and URLs, introduces Media Input Rules section specifying cwd-relative path validation, broadens parameter descriptions to document path|url|key inputs, and clarifies automatic uploading behavior.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Suggested labels

domain/im, size/L

Suggested reviewers

  • fangshuyu-768
  • liangshuo-1

Poem

🐰 Media paths once cryptic and obscure,
Now documented for all to endure,
Keys, URLs, cwd-relative rules so clear,
Constants shared, no more secrets here,
Tests enforce the truth we hold dear!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'docs(im): clarify media path restrictions' accurately summarizes the main change: documentation clarification of media flag path restrictions.
Description check ✅ Passed The description follows the template with all required sections: Summary, Changes, Test Plan, and Related Issues. All sections are complete and provide clear details.
Linked Issues check ✅ Passed The PR fully addresses issue #872 by documenting the cwd-relative path restriction in flag descriptions, help text, and reference docs, and adding test coverage for the documentation.
Out of Scope Changes check ✅ Passed All changes are directly related to clarifying media path restrictions: sharing flag descriptions, updating docs, and adding test coverage. No unrelated changes detected.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 19, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.78%. Comparing base (315e0ab) to head (759eb62).
⚠️ Report is 12 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #956   +/-   ##
=======================================
  Coverage   66.78%   66.78%           
=======================================
  Files         564      564           
  Lines       52441    52441           
=======================================
  Hits        35024    35024           
  Misses      14516    14516           
  Partials     2901     2901           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@759eb622a2838906db14e145da1d025317833d18

🧩 Skill update

npx skills add mtsui-cmyk/cli#docs/im-media-path-help -y -g

@YangJunzhou-01
Copy link
Copy Markdown
Collaborator

Thanks for the PR! This is a point worthy of optimization, a couple of suggestions:

  1. Avoid static variables for Desc fields — Static constants may affect non-runtime flows. To keep code style consistent with the rest of the codebase, please inline the description strings directly in the flag definitions rather than referencing shared constants.
  2. Unify description prefix style — The image flag uses "image_key, URL, or ..." while video uses "video file_key, URL, or ...". Consider making the pattern uniform by always including the type and key format example:
// Suggestion: unify prefix style
imImageFlagDesc      = "image key (img_xxx), URL, or cwd-relative local path (…)"
imFileFlagDesc       = "file key (file_xxx), URL, or cwd-relative local path (…)"
imVideoFlagDesc      = "video file key (file_xxx), URL, or cwd-relative local path (…); must be used together with --video-cover"
//...

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

Labels

domain/im PR touches the im domain size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--image / --file flags reject absolute paths but --help doesn't document this

3 participants