Skip to content

fix(lark): contain panics in asynchronous card actions - #177

Open
kevinmatthe wants to merge 1 commit into
masterfrom
codex/fix-card-refresh-action-vulnerability
Open

fix(lark): contain panics in asynchronous card actions#177
kevinmatthe wants to merge 1 commit into
masterfrom
codex/fix-card-refresh-action-vulnerability

Conversation

@kevinmatthe

@kevinmatthe kevinmatthe commented Jul 31, 2026

Copy link
Copy Markdown
Member

Motivation

  • Card "refresh" actions replay embedded commands in a newly spawned goroutine without a local recover boundary, allowing malformed/partial synthetic events to panic and terminate the whole process.
  • The change ensures asynchronous card action tasks cannot crash the bot and provides actionable logs for operators when a panic occurs.

Description

  • Route async card action tasks through a safe wrapper by replacing the direct go task(context.WithoutCancel(ctx)) call with go runAsyncTask(context.WithoutCancel(ctx), action.Name, task) in Dispatch (internal/application/lark/cardaction/registry.go).
  • Add runAsyncTask(ctx, action, task) which defers a recover() and logs the recovered value plus the stack trace using logs.L().Ctx(ctx).Error(...) so panics are contained and observable.
  • Add imports required for the wrapper (runtime/debug, github.com/BetaGoRobot/BetaGo-Redefine/pkg/logs, go.uber.org/zap).
  • Add a regression test TestRunAsyncTaskRecoversPanic in internal/application/lark/cardaction/registry_test.go that verifies a panic inside the async task does not escape the goroutine boundary.

Testing

  • Ran git diff --check and applied gofmt -w on the modified files (both succeeded).
  • Added and committed the changes (fix(lark): recover async card action panics).
  • Attempted go test ./internal/application/lark/cardaction ./internal/interfaces/lark but full test execution is blocked in this environment due to the native libvips/bimg dependency required by other packages, so package-level integration tests could not be completed here.
  • Attempted CGO_ENABLED=0 go test ./internal/application/lark/cardaction -run TestRunAsyncTaskRecoversPanic but running tests in this container is constrained by native/CGO dependencies; the regression test was added and will run in CI where native dependencies are available.

Codex Task

Summary by Sourcery

在异步 Lark 卡片操作任务中捕获 panic,防止其导致机器人崩溃,并改为产生可观察的错误日志。

Bug Fixes(错误修复):

  • 确保异步卡片操作的 goroutine 能从 panic 中恢复,并记录失败日志,而不是终止整个进程。

Enhancements(增强):

  • 为异步卡片操作中被捕获的 panic 添加结构化日志,包括操作名称和堆栈追踪,以便更容易诊断问题。

Tests(测试):

  • 添加回归测试,验证 runAsyncTask 能够包含 panic,并阻止它们逃离 goroutine 边界。
Original summary in English

Summary by Sourcery

Contain panics in asynchronous Lark card action tasks so they cannot crash the bot and instead produce observable error logs.

Bug Fixes:

  • Ensure async card action goroutines recover from panics and log the failure instead of terminating the process.

Enhancements:

  • Add structured logging of recovered panics from async card actions, including action name and stack trace for easier diagnosis.

Tests:

  • Add a regression test verifying that runAsyncTask contains panics and prevents them from escaping the goroutine boundary.

@sourcery-ai

sourcery-ai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Reviewer's Guide

添加一个对异步卡片操作任务进行 panic 保护的封装,以及一个回归测试,用于确保 panic 被限制并记录日志,而不是导致进程崩溃。

Panic 安全的异步卡片操作执行序列图

sequenceDiagram
  actor User
  participant LarkCallback as CardActionDispatch
  participant AsyncTask as AsyncTask
  participant Logger as logs

  User->>LarkCallback: Dispatch(ctx, event, metaData)
  LarkCallback->>AsyncTask: runAsyncTask(context.WithoutCancel(ctx), action.Name, task)
  activate AsyncTask
  AsyncTask->>AsyncTask: task(ctx)
  alt [panic occurs]
    AsyncTask->>Logger: logs.L().Ctx(ctx).Error("panic in async card action")
  end
  deactivate AsyncTask
Loading

File-Level Changes

Change Details Files
使用一个带 panic 恢复的辅助函数封装异步卡片操作任务,在发生 panic 时记录详细信息,而不是让它们导致机器人崩溃。
  • 将 Dispatch 中直接以 goroutine 方式调用异步任务的方式替换为调用新的 runAsyncTask 辅助函数,并传入 action 名称用于日志记录。
  • 实现 runAsyncTask,在其中通过 defer recover 检测非 nil 的 panic,并使用共享的日志基础设施记录 action、panic 值和堆栈信息。
  • 添加用于获取调试堆栈和结构化日志依赖所需的 import。
internal/application/lark/cardaction/registry.go
添加一个回归测试,以确保 runAsyncTask 将 panic 限制在 goroutine 边界内。
  • 创建 TestRunAsyncTaskRecoversPanic,使用一个会发生 panic 的任务调用 runAsyncTask,并验证控制流能够正常返回。
  • 使用完成标志和外层 defer 来断言 panic 不会逃逸出 goroutine 的边界。
internal/application/lark/cardaction/registry_test.go

Tips and commands

Interacting with Sourcery

  • 触发新的代码审查: 在 Pull Request 中评论 @sourcery-ai review
  • 继续讨论: 直接回复 Sourcery 的审查评论。
  • 从审查评论生成 GitHub issue: 通过回复审查评论来请求 Sourcery 从该评论创建一个 issue。你也可以在审查评论下回复 @sourcery-ai issue 来从该评论创建 issue。
  • 生成 Pull Request 标题: 在 Pull Request 标题的任意位置写入 @sourcery-ai,即可随时生成标题。你也可以在 Pull Request 中评论 @sourcery-ai title 来(重新)生成标题。
  • 生成 Pull Request 摘要: 在 Pull Request 描述正文的任意位置写入 @sourcery-ai summary,即可在指定位置随时生成 PR 摘要。你也可以在 Pull Request 中评论 @sourcery-ai summary 来(重新)生成摘要。
  • 生成审查者指南: 在 Pull Request 中评论 @sourcery-ai guide,即可随时(重新)生成审查者指南。
  • 解决所有 Sourcery 评论: 在 Pull Request 中评论 @sourcery-ai resolve,可以一次性标记解决所有 Sourcery 评论。如果你已经处理了所有评论且不再希望看到它们,这会很有用。
  • 关闭所有 Sourcery 审查: 在 Pull Request 中评论 @sourcery-ai dismiss,可以关闭所有现有的 Sourcery 审查。特别适用于你想从一个全新的审查开始——别忘了评论 @sourcery-ai review 来触发新的审查!

Customizing Your Experience

访问你的控制面板 来:

  • 启用或禁用审查功能,例如 Sourcery 生成的 Pull Request 摘要、审查者指南等。
  • 更改审查语言。
  • 添加、删除或编辑自定义审查说明。
  • 调整其他审查设置。

Getting Help

Original review guide in English

Reviewer's Guide

Adds a panic-safe wrapper around asynchronous card action tasks and a regression test to ensure panics are contained and logged instead of crashing the process.

Sequence diagram for panic-safe async card action execution

sequenceDiagram
  actor User
  participant LarkCallback as CardActionDispatch
  participant AsyncTask as AsyncTask
  participant Logger as logs

  User->>LarkCallback: Dispatch(ctx, event, metaData)
  LarkCallback->>AsyncTask: runAsyncTask(context.WithoutCancel(ctx), action.Name, task)
  activate AsyncTask
  AsyncTask->>AsyncTask: task(ctx)
  alt [panic occurs]
    AsyncTask->>Logger: logs.L().Ctx(ctx).Error("panic in async card action")
  end
  deactivate AsyncTask
Loading

File-Level Changes

Change Details Files
Wrap asynchronous card action tasks in a panic-recovering helper that logs panic details instead of letting them crash the bot.
  • Replace direct goroutine invocation of async tasks in Dispatch with a call to a new runAsyncTask helper, passing the action name for logging.
  • Implement runAsyncTask to defer a recover, detect non-nil panics, and log the action, panic value, and stack trace using the shared logging infrastructure.
  • Add required imports for debug stack capture and structured logging dependencies.
internal/application/lark/cardaction/registry.go
Add a regression test to ensure runAsyncTask contains panics within the goroutine boundary.
  • Create TestRunAsyncTaskRecoversPanic that invokes runAsyncTask with a task that panics and verifies control flow returns normally.
  • Use a completion flag and outer defer to assert the panic does not escape the goroutine boundary.
internal/application/lark/cardaction/registry_test.go

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

嗨,我已经审查了你的修改,整体看起来很棒!


Sourcery 对开源项目是免费的——如果你觉得我们的审查有帮助,欢迎分享 ✨
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进之后的审查。
Original comment in English

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant