Skip to content

chore(sync): merge upstream main back into dev - #26

Merged
JOY (JOY) merged 5 commits into
devfrom
sync/upstream-main-2026-09-25
Oct 1, 2026
Merged

JOY (JOY) merged 5 commits into
devfrom
sync/upstream-main-2026-09-25

Conversation

@JOY

@JOY JOY (JOY) commented Oct 1, 2026 •

Copy link
Copy Markdown

Summary

Periodic sync after upstream merged huabeitech#51 (the Slack deployment-wide credentials + webhook hardening port, originally authored here) and huabeitech#52 (community docs PR - English screenshots).

Conflict resolution notes:

  • the fork's Slack implementation and the upstream feat(slack): deployment-wide Slack app credentials and webhook hardening huabeitech/agent-desk#51 port descend from the same source, so conflicts were drift-level; the fork side keeps its hardened variants (fail-closed verification token check, secret-source warn logs, replay-window tests)
  • union kept BOTH deployment-fallback tests (the upstream-port's TestSlackInboundFallsBackToDeploymentSigningSecret and the fork's TestVerifySlackSignatureEnforcesReplayWindow drift table)
  • firstNonBlank and the config resolver suite deduped; env example kept the fork's fuller Meta/Slack documentation blocks

No upstream file or test dropped (verified post-commit). DB_TYPE stays out of prod .env per the 2026-09-25 sqlite incident.

Validation

Full Go suite green with go test -tags dev across the CI package list; vet clean; frontend typecheck clean.


📌 TL;DR

This PR implements a fallback mechanism for Slack webhook signature verification, allowing channels without a specific signing secret to use the deployment-wide configuration. It also includes minor import and whitespace cleanups in related Slack service files.

🎯 Type of Change

  • 🚀 New feature
  • 🐛 Bugfix
  • 🧹 Refactor
  • ⚡ Performance
  • 📚 Documentation
  • ⚙️ CI / Configuration

🔍 Changes Walkthrough

File Summary of Changes
internal/services/slack_inbound_service.go Reorganized imports to group standard library, internal models, and internal packages logically.
internal/services/slack_inbound_service_test.go Added a new test case TestSlackInboundFallsBackToDeploymentSigningSecret to verify that webhooks signed with the global deployment secret are accepted when a channel lacks a specific secret.
internal/services/slack_outbound_service.go Removed unnecessary blank lines in imports and within the processOutbox function for code cleanliness.

📊 Architectural Flow

sequenceDiagram
    participant Slack as Slack Platform
    participant Service as SlackInboundService
    participant Channel as Channel Model
    participant Config as Global Config

    Slack->>Service: Webhook Request (Timestamp, Signature, Payload)
    Service->>Channel: Get Channel by ID
    alt Channel has specific SigningSecret
        Service->>Service: Verify Signature using Channel Secret
    else Channel SigningSecret is empty
        Service->>Config: Get Deployment-wide SigningSecret
        Service->>Service: Verify Signature using Deployment Secret
    end
    Service->>Service: Process Event if Valid
Loading

JOY (JOY) and others added 5 commits September 23, 2026 13:57
A Slack channel currently has to carry its own bot token and signing
secret, so a deployment serving several Slack workspaces must create one
app per channel, and a channel whose secret is absent silently accepts
unsigned deliveries.

- config gains deployment-wide slack.clientId/clientSecret/botToken/
  signingSecret (SLACK_* environment aliases follow the existing Discord
  pattern); config.ResolveSlack resolves per channel with the
  deployment-wide values as the fallback
- the inbound webhook verifies deliveries against whatever secret
  resolves - channel first, deployment-wide second - and warn-logs every
  rejected delivery with the channel id and where the verifying secret
  came from, so a channel bound to a second Slack app is diagnosable the
  moment the fallback secret appears
- outbound replies fall back to the deployment-wide bot token the same
  way, so a shared app can post on behalf of every channel
- tests: SlackApp precedence, ResolveSlack before Load, and an inbound
  delivery signed with the deployment secret being accepted on a channel
  that carries none
feat(slack): deployment-wide Slack app credentials and webhook hardening
Add English UI screenshots under screenshots/en/ and point the English
README.md at them, so README.md shows English UI while README_ZH.md keeps
the original Chinese screenshots.

Co-authored-by: Confetti Labs <confetti-labs@users.noreply.github.com>
…2026-09-25

# Conflicts:
#	.env.example
#	internal/pkg/config/config.go
#	internal/pkg/config/runtime.go
#	internal/services/slack_inbound_service_test.go
#	internal/services/slack_outbound_service.go
@coderabbitai

coderabbitai Bot commented Oct 1, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 3e925da8-e040-4927-bcb9-a973351aa70d

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Autopilot is currently an internal CodeRabbit preview.


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.

@JOY
JOY (JOY) merged commit 6ebb230 into dev Oct 1, 2026
6 checks passed

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request updates screenshot asset paths in the README, cleans up imports and whitespace in the Slack services, and adds a new test to verify that Slack inbound channels fall back to the deployment-wide signing secret. The feedback recommends capturing and restoring the existing global configuration in the test's defer block rather than setting it to nil, preventing potential side effects on other tests.

Comment on lines +389 to +390
config.SetCurrent(&config.Config{Slack: config.SlackConfig{SigningSecret: slackTestSigningSecret}})
defer config.SetCurrent(nil)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Unconditionally setting the global configuration to nil in defer can break other tests in the same package if they run sequentially and rely on a previously initialized configuration. It is safer to capture the existing configuration and restore it when the test finishes.

Suggested change
config.SetCurrent(&config.Config{Slack: config.SlackConfig{SigningSecret: slackTestSigningSecret}})
defer config.SetCurrent(nil)
oldConfig := config.GetCurrent()
config.SetCurrent(&config.Config{Slack: config.SlackConfig{SigningSecret: slackTestSigningSecret}})
defer config.SetCurrent(oldConfig)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants