Add Rocket.Chat provider - #545
Conversation
WalkthroughAdds Notion and Rocket.Chat providers, changes Slack webhook delivery to direct HTTP, improves runner configuration initialization, updates HTTP and dependency handling, and removes the integration-test harness. ChangesProvider integrations
Slack webhook delivery
Runner configuration
Maintenance and cleanup
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Notify
participant ProviderRegistry
participant Notion
participant RocketChat
participant ExternalService
Notify->>ProviderRegistry: Initialize configured providers
ProviderRegistry->>Notion: Register selected destinations
ProviderRegistry->>RocketChat: Register selected destinations
Notion->>ExternalService: Create database page
RocketChat->>ExternalService: POST webhook payload
ExternalService-->>Notion: Return API response
ExternalService-->>RocketChat: Return HTTP status
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@pkg/providers/rocketchat/rocketchat.go`:
- Around line 52-88: Update Provider.Send to track whether any chunk fails for
each provider, resetting the flag at the start of each Rocketchat provider
iteration and setting it when send returns an error; emit the existing success
log only when that provider’s chunks all succeed, while preserving error
accumulation and continuation behavior.
- Around line 90-106: Update send to avoid http.Post by creating a context-aware
POST request and executing it with an HTTP client configured with a finite
timeout. Preserve the existing JSON payload and non-success status handling, use
Client.Do with NewRequestWithContext, and handle any error from resp.Body.Close
so the noctx and errcheck findings are resolved.
- Around line 108-145: Update splitMessage to measure message and line lengths
in runes and slice via rune boundaries, ensuring multi-byte UTF-8 characters are
never split while preserving the existing newline-aware chunking behavior. Add a
multi-byte content case, such as emoji or accented text, to TestSplitMessage
covering correct chunk contents and limits.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 40375ab7-7eb5-47a4-a607-250ab571bfed
📒 Files selected for processing (4)
pkg/providers/providers.gopkg/providers/rocketchat/rocketchat.gopkg/providers/rocketchat/rocketchat_test.gopkg/providers/rocketchat/rocketchat_types.go
Adds a new notify provider for Rocket.Chat Incoming Webhooks, supporting configurable channel, username (alias), avatar and emoji, plain-text or attachment-based messages, and automatic splitting of messages larger than Rocket.Chat's default max message size.
Use bounded HTTP requests, preserve UTF-8 message chunks, and avoid reporting success after partial delivery failures.
cea9538 to
59e9d3b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
pkg/providers/notion/notion.go (1)
19-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse "ID" instead of "Id" for initialisms in exported field names. Both sites introduce a new struct field ending in "Id" instead of the Go convention "ID", and
notion.go'sOptionsstruct now mixes both conventions in the same declaration (IDat line 20 vsNotionDatabaseIdat line 22).
pkg/providers/notion/notion.go#L19-L24: renameNotionDatabaseIdtoNotionDatabaseIDfor consistency with the siblingIDfield; keep theyaml:"notion_database_id,omitempty"tag unchanged.pkg/providers/notion/notion_types.go#L1-L21: renamePage.ParentIdtoPage.ParentID; keep thejson:"parent_id"tag unchanged, and update the one usage inpkg/providers/notion/notion_api.go(page.ParentId) accordingly.🤖 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 `@pkg/providers/notion/notion.go` around lines 19 - 24, Rename Options.NotionDatabaseId to NotionDatabaseID in pkg/providers/notion/notion.go, preserving the existing YAML tag. Rename Page.ParentId to ParentID in pkg/providers/notion/notion_types.go, preserve its JSON tag, and update the page.ParentId usage in pkg/providers/notion/notion_api.go to page.ParentID.
🤖 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 `@go.mod`:
- Line 96: Update the golang.org/x/crypto dependency in go.mod from v0.46.0 to
v0.52.0, then refresh the Go module graph so go.sum and any indirect dependency
versions remain consistent.
In `@internal/runner/runner.go`:
- Around line 66-70: Update the os.WriteFile error branch in the runner
initialization flow to return the writeErr, wrapped with the surrounding
operation context, instead of logging the warning and continuing. Preserve the
existing success logging in the else branch so NewRunner cannot succeed with an
incomplete provider configuration.
In `@pkg/providers/notion/notion_api.go`:
- Around line 28-36: Update the response handling after httpreq.NewClient().Post
in the Notion request flow to check whether response is non-nil before accessing
response.Object or response.Message. Only validate the Notion response fields
when a decoded response exists, while preserving the existing error return for
non-page responses.
In `@pkg/providers/notion/notion.go`:
- Around line 53-58: Update the error branch in the CreatePage call to rename
success to an error-appropriate variable and wrap that underlying error with the
existing ID context before appending it to NotionErr. Remove the generic
fmt.Errorf that discards the CreatePage failure details, while preserving the
continue behavior.
In `@pkg/providers/slack/slack.go`:
- Around line 101-105: Update the Slack webhook request in Provider.Send to use
an http.Client with a 10-second timeout instead of the implicit http.Post
client; if Send accepts a context.Context, build the request with that context
and execute it via the client’s Do method, while preserving the existing error
wrapping and multierr handling.
---
Nitpick comments:
In `@pkg/providers/notion/notion.go`:
- Around line 19-24: Rename Options.NotionDatabaseId to NotionDatabaseID in
pkg/providers/notion/notion.go, preserving the existing YAML tag. Rename
Page.ParentId to ParentID in pkg/providers/notion/notion_types.go, preserve its
JSON tag, and update the page.ParentId usage in
pkg/providers/notion/notion_api.go to page.ParentID.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4ff7ff7-c37c-4f6b-b76a-63a91189f796
⛔ Files ignored due to path filters (9)
.github/workflows/build-test.ymlis excluded by!**/*.yml.github/workflows/codeql-analysis.ymlis excluded by!**/*.yml.github/workflows/dockerhub-push.ymlis excluded by!**/*.yml.github/workflows/lint-test.ymlis excluded by!**/*.yml.github/workflows/release-binary.ymlis excluded by!**/*.yml.github/workflows/release-test.ymlis excluded by!**/*.yml.goreleaser.ymlis excluded by!**/*.ymlcmd/integration-test/test-config.yamlis excluded by!**/*.yamlgo.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
README.mdcmd/integration-test/action-run.shcmd/integration-test/gotify.shcmd/integration-test/integration.gocmd/integration-test/providers.gocmd/integration-test/run.shgo.modinternal/runner/runner.gointernal/runner/runner_test.gointernal/testutils/integration.gopkg/providers/notion/notion.gopkg/providers/notion/notion_api.gopkg/providers/notion/notion_types.gopkg/providers/providers.gopkg/providers/rocketchat/rocketchat.gopkg/providers/rocketchat/rocketchat_test.gopkg/providers/rocketchat/rocketchat_types.gopkg/providers/slack/slack.gopkg/utils/httpreq/httpreq.go
💤 Files with no reviewable changes (6)
- cmd/integration-test/gotify.sh
- cmd/integration-test/action-run.sh
- cmd/integration-test/run.sh
- cmd/integration-test/providers.go
- internal/testutils/integration.go
- cmd/integration-test/integration.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/providers/rocketchat/rocketchat_types.go
- pkg/providers/rocketchat/rocketchat.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 5
🧹 Nitpick comments (1)
pkg/providers/notion/notion.go (1)
19-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUse "ID" instead of "Id" for initialisms in exported field names. Both sites introduce a new struct field ending in "Id" instead of the Go convention "ID", and
notion.go'sOptionsstruct now mixes both conventions in the same declaration (IDat line 20 vsNotionDatabaseIdat line 22).
pkg/providers/notion/notion.go#L19-L24: renameNotionDatabaseIdtoNotionDatabaseIDfor consistency with the siblingIDfield; keep theyaml:"notion_database_id,omitempty"tag unchanged.pkg/providers/notion/notion_types.go#L1-L21: renamePage.ParentIdtoPage.ParentID; keep thejson:"parent_id"tag unchanged, and update the one usage inpkg/providers/notion/notion_api.go(page.ParentId) accordingly.🤖 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 `@pkg/providers/notion/notion.go` around lines 19 - 24, Rename Options.NotionDatabaseId to NotionDatabaseID in pkg/providers/notion/notion.go, preserving the existing YAML tag. Rename Page.ParentId to ParentID in pkg/providers/notion/notion_types.go, preserve its JSON tag, and update the page.ParentId usage in pkg/providers/notion/notion_api.go to page.ParentID.
🤖 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 `@go.mod`:
- Line 96: Update the golang.org/x/crypto dependency in go.mod from v0.46.0 to
v0.52.0, then refresh the Go module graph so go.sum and any indirect dependency
versions remain consistent.
In `@internal/runner/runner.go`:
- Around line 66-70: Update the os.WriteFile error branch in the runner
initialization flow to return the writeErr, wrapped with the surrounding
operation context, instead of logging the warning and continuing. Preserve the
existing success logging in the else branch so NewRunner cannot succeed with an
incomplete provider configuration.
In `@pkg/providers/notion/notion_api.go`:
- Around line 28-36: Update the response handling after httpreq.NewClient().Post
in the Notion request flow to check whether response is non-nil before accessing
response.Object or response.Message. Only validate the Notion response fields
when a decoded response exists, while preserving the existing error return for
non-page responses.
In `@pkg/providers/notion/notion.go`:
- Around line 53-58: Update the error branch in the CreatePage call to rename
success to an error-appropriate variable and wrap that underlying error with the
existing ID context before appending it to NotionErr. Remove the generic
fmt.Errorf that discards the CreatePage failure details, while preserving the
continue behavior.
In `@pkg/providers/slack/slack.go`:
- Around line 101-105: Update the Slack webhook request in Provider.Send to use
an http.Client with a 10-second timeout instead of the implicit http.Post
client; if Send accepts a context.Context, build the request with that context
and execute it via the client’s Do method, while preserving the existing error
wrapping and multierr handling.
---
Nitpick comments:
In `@pkg/providers/notion/notion.go`:
- Around line 19-24: Rename Options.NotionDatabaseId to NotionDatabaseID in
pkg/providers/notion/notion.go, preserving the existing YAML tag. Rename
Page.ParentId to ParentID in pkg/providers/notion/notion_types.go, preserve its
JSON tag, and update the page.ParentId usage in
pkg/providers/notion/notion_api.go to page.ParentID.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e4ff7ff7-c37c-4f6b-b76a-63a91189f796
⛔ Files ignored due to path filters (9)
.github/workflows/build-test.ymlis excluded by!**/*.yml.github/workflows/codeql-analysis.ymlis excluded by!**/*.yml.github/workflows/dockerhub-push.ymlis excluded by!**/*.yml.github/workflows/lint-test.ymlis excluded by!**/*.yml.github/workflows/release-binary.ymlis excluded by!**/*.yml.github/workflows/release-test.ymlis excluded by!**/*.yml.goreleaser.ymlis excluded by!**/*.ymlcmd/integration-test/test-config.yamlis excluded by!**/*.yamlgo.sumis excluded by!**/*.sum
📒 Files selected for processing (19)
README.mdcmd/integration-test/action-run.shcmd/integration-test/gotify.shcmd/integration-test/integration.gocmd/integration-test/providers.gocmd/integration-test/run.shgo.modinternal/runner/runner.gointernal/runner/runner_test.gointernal/testutils/integration.gopkg/providers/notion/notion.gopkg/providers/notion/notion_api.gopkg/providers/notion/notion_types.gopkg/providers/providers.gopkg/providers/rocketchat/rocketchat.gopkg/providers/rocketchat/rocketchat_test.gopkg/providers/rocketchat/rocketchat_types.gopkg/providers/slack/slack.gopkg/utils/httpreq/httpreq.go
💤 Files with no reviewable changes (6)
- cmd/integration-test/gotify.sh
- cmd/integration-test/action-run.sh
- cmd/integration-test/run.sh
- cmd/integration-test/providers.go
- internal/testutils/integration.go
- cmd/integration-test/integration.go
🚧 Files skipped from review as they are similar to previous changes (2)
- pkg/providers/rocketchat/rocketchat_types.go
- pkg/providers/rocketchat/rocketchat.go
🛑 Comments failed to post (5)
go.mod (1)
96-96: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail curl -sS https://api.osv.dev/v1/query \ -H 'Content-Type: application/json' \ -d '{"package":{"ecosystem":"Go","name":"golang.org/x/crypto"},"version":"v0.46.0"}' | jq '{vulnerabilities: [.vulns[] | {id, summary, modified, affected}]}'Repository: projectdiscovery/notify
Length of output: 30296
🏁 Script executed:
#!/bin/bash set -euo pipefail printf 'go.mod crypto line:\n' rg -n 'golang.org/x/crypto' go.mod || true printf '\nmodule graph references mentioning ssh/agent/openpgp:\n' rg -n '"golang.org/x/crypto/(ssh/(agent|knownhosts)|openpgp)' -g '*.go' . || true printf '\nmodule availability summary:\n' version='v0.52.0' echo "module golang.org/x/crypto $version" > /tmp/mod_check.go.mod go get -modfile=/tmp/mod_check.go.mod golang.org/x/crypto@$version 2>&1 || true rm -f /tmp/mod_check.go.sum /tmp/mod_check.go.modRepository: projectdiscovery/notify
Length of output: 403
🌐 Web query:
golang.org/x/crypto v0.52.0 module go proxy💡 Result:
The module golang.org/x/crypto version v0.52.0 is a legitimate, tagged release of the Go supplementary cryptography library [1][2]. It was published to address specific security vulnerabilities [3]. You can retrieve and use this version through the standard Go module proxy (proxy.golang.org) by running the following command in your terminal: go get golang.org/x/crypto@v0.52.0 As an official part of the Go project's "x" (extended) repository, it is indexed and served by the Go module proxy, which provides an immutable, reliable source for Go modules [4][5]. As of August 3, 2026, newer versions (such as v0.54.0 and later) are also available, but v0.52.0 remains accessible via the proxy [4][6][7].
Citations:
- 1: https://pkg.go.dev/golang.org/x/crypto@v0.52.0
- 2: https://go.googlesource.com/crypto/+/refs/tags/v0.52.0
- 3: https://groups.google.com/g/golang-announce/c/a082jnz-LvI
- 4: https://pkg.go.dev/golang.org/x/crypto
- 5: https://github.com/golang/crypto
- 6: https://pkg.go.dev/golang.org/x/crypto@v0.31.0
- 7: https://pkg.go.dev/golang.org/x/crypto@v0.45.0
Upgrade
golang.org/x/cryptoto a fixed version.
golang.org/x/crypto v0.46.0is affected by multiple advisories fixed inv0.52.0, including SSH key enforcement and FIDO/U2F checks. Update the dependency and refresh the module graph.🧰 Tools
🪛 OSV Scanner (2.4.0)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking key constraints not enforced in golang.org/x/crypto/ssh/agent
(GO-2026-5005)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking agent constraints dropped when forwarding keys in golang.org/x/crypto/ssh/agent
(GO-2026-5006)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking byte arithmetic causes underflow and panic in golang.org/x/crypto/ssh
(GO-2026-5013)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking bypass of certificate restrictions in golang.org/x/crypto/ssh
(GO-2026-5014)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking server panic during CheckHostKey/Authenticate in golang.org/x/crypto/ssh
(GO-2026-5015)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking memory leak when rejecting channels can lead to DoS in golang.org/x/crypto/ssh
(GO-2026-5016)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking client can cause server deadlock on unexpected responses in golang.org/x/crypto/ssh
(GO-2026-5017)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking pathological RSA/DSA parameters may cause DoS in golang.org/x/crypto/ssh
(GO-2026-5018)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking bypass of FIDO/U2F security keys physical interaction in golang.org/x/crypto/ssh
(GO-2026-5019)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking infinite loop on large channel writes in golang.org/x/crypto/ssh
(GO-2026-5020)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking auth bypass via unenforced
@revokedstatus in golang.org/x/crypto/ssh/knownhosts(GO-2026-5021)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking VerifiedPublicKeyCallback permissions skip enforcement in golang.org/x/crypto/ssh
(GO-2026-5023)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: Invoking pathological inputs can lead to client panic in golang.org/x/crypto/ssh/agent
(GO-2026-5033)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: The golang.org/x/crypto/openpgp package is unmaintained, unsafe by design, and has known security issues
(GO-2026-5932)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto vulnerable to invoking bypass of certificate restrictions
(GHSA-45gg-vh54-h5m9)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto vulnerable to auth bypass via unenforced
@revokedstatus(GHSA-5cgq-3rg8-m6cv)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto is vulnerable to invoking server panic during CheckHostKey/Authenticate flow
(GHSA-78mq-xcr3-xm33)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: FIDO/U2F security key physical presence check can be bypassed
(GHSA-89gr-r52h-f8rx)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: Invoking pathological inputs can lead to client panic
(GHSA-9m57-25v3-79x9)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto doesn't drop invoking agent constraints when forwarding keys
(GHSA-f5wc-c3c7-36mc)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto doesn't enforce invoking key constraints
(GHSA-jppx-rxg9-jmrx)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: Invoking byte arithmetic causes underflow and panic
(GHSA-q4h4-gmj2-qvw2)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: Invoking memory leak when rejecting channels can lead to DoS
(GHSA-qpw4-5x99-6vjp)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto vulnerable to infinite loop on large channel writes
(GHSA-rm3j-f69w-wqmq)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: Invoking client can cause server deadlock on unexpected responses
(GHSA-vgwf-h737-ff37)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: Invoking pathological RSA/DSA parameters may cause DoS
(GHSA-w879-237q-wc7r)
[CRITICAL] 96-96: golang.org/x/crypto 0.46.0: golang.org/x/crypto: Invoking VerifiedPublicKeyCallback permissions skip enforcement
(GHSA-x527-x647-q7gg)
🤖 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 `@go.mod` at line 96, Update the golang.org/x/crypto dependency in go.mod from v0.46.0 to v0.52.0, then refresh the Go module graph so go.sum and any indirect dependency versions remain consistent.Source: Linters/SAST tools
internal/runner/runner.go (1)
66-70: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Return template write errors.
If
os.WriteFilefails after a partial write, the file can still contain only comments. Lines 81-84 then accept the file, andNewRunnercan return success with no configured providers. Return the wrappedwriteErrinstead of continuing.Proposed fix
if writeErr := os.WriteFile(options.ProviderConfig, []byte(defaultConfig), 0600); writeErr != nil { - gologger.Warning().Msgf("Could not create default provider config at %s: %s\n", options.ProviderConfig, writeErr) + return nil, errors.Wrap(writeErr, "could not create default provider config") } else { gologger.Info().Msgf("Created default provider config at %s — please add your provider credentials.\n", options.ProviderConfig) }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if writeErr := os.WriteFile(options.ProviderConfig, []byte(defaultConfig), 0600); writeErr != nil { return nil, errors.Wrap(writeErr, "could not create default provider config") } else { gologger.Info().Msgf("Created default provider config at %s — please add your provider credentials.\n", options.ProviderConfig) }🤖 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 `@internal/runner/runner.go` around lines 66 - 70, Update the os.WriteFile error branch in the runner initialization flow to return the writeErr, wrapped with the surrounding operation context, instead of logging the warning and continuing. Preserve the existing success logging in the else branch so NewRunner cannot succeed with an incomplete provider configuration.pkg/providers/notion/notion_api.go (1)
28-36: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Inspect httpreq.Post to confirm decode-on-nil-error behavior. fd -a 'httpreq.go' pkg/utils/httpreq rg -n -B2 -A40 'func.*Post' pkg/utils/httpreq/httpreq.goRepository: projectdiscovery/notify
Length of output: 1115
Decode non-empty responses before checking the Notion response fields.
Postalways decodes the response when the request succeeds, but empty response bodies still pass backnilerror. Add an explicit response-body check and only validateresponse.Objectwhen the decoded body is non-empty to avoid anilResponsedereference.🤖 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 `@pkg/providers/notion/notion_api.go` around lines 28 - 36, Update the response handling after httpreq.NewClient().Post in the Notion request flow to check whether response is non-nil before accessing response.Object or response.Message. Only validate the Notion response fields when a decoded response exists, while preserving the existing error return for non-page responses.pkg/providers/notion/notion.go (1)
53-58: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Do not discard the underlying error from
CreatePage.
successholds the actual error returned bypr.CreatePage(newPage), which can contain the Notion API failure message (seenotion_api.goline 35:"error while sending notion message: %s ") or a network error. The code replaces it with a new, genericfmt.Errorf("failed to send notion notification")and discardssuccessentirely. This removes all diagnostic detail needed to debug a failed send.Wrap the actual error instead of creating a new one, and rename the misleadingly-named
successvariable, since it holds an error, not a success flag.🐛 Proposed fix to preserve the underlying error
- if success := pr.CreatePage(newPage); success != nil { - err := errors.Wrap(fmt.Errorf("failed to send notion notification"), - fmt.Sprintf("failed to send notion notification for id: %s", pr.ID)) - NotionErr = multierr.Append(NotionErr, err) - continue - } + if err := pr.CreatePage(newPage); err != nil { + wrappedErr := errors.Wrap(err, fmt.Sprintf("failed to send notion notification for id: %s", pr.ID)) + NotionErr = multierr.Append(NotionErr, wrappedErr) + continue + }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.if err := pr.CreatePage(newPage); err != nil { wrappedErr := errors.Wrap(err, fmt.Sprintf("failed to send notion notification for id: %s", pr.ID)) NotionErr = multierr.Append(NotionErr, wrappedErr) continue }🤖 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 `@pkg/providers/notion/notion.go` around lines 53 - 58, Update the error branch in the CreatePage call to rename success to an error-appropriate variable and wrap that underlying error with the existing ID context before appending it to NotionErr. Remove the generic fmt.Errorf that discards the CreatePage failure details, while preserving the continue behavior.pkg/providers/slack/slack.go (1)
101-105: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail # Inspect the provider Send contract and its callers before adding context propagation. ast-grep outline pkg/providers/slack/slack.go --items all rg -n -C 4 --type go 'func \(p \*Provider\) Send\(|\.Send\(' .Repository: projectdiscovery/notify
Length of output: 16126
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== slack provider ==" cat -n pkg/providers/slack/slack.go | sed -n '1,140p' echo "== providers interface and Send aggregation ==" cat -n pkg/providers/providers.go | sed -n '1,220p' echo "== runner Send path ==" cat -n internal/runner/runner.go | sed -n '130,195p' echo "== http utility clients/defaults ==" fd -a httpreq.go pkg/util pkg/utils . | sed 's#^\./##' | xargs -r -n1 sh -c 'echo "--- $0"; cat -n "$0" | sed -n "1,180p"' echo "== search for Client default / HTTP client uses ==" rg -n --type go 'http\.Client|Client:|Do\(|Timeout:' .Repository: projectdiscovery/notify
Length of output: 18859
Bound the Slack webhook request duration.
http.Postuses the implicit client, which has no timeout; a stalled Slack webhook can blockProvider.Send. Use a 10-secondhttp.Clientfor this request. IfProvider.Sendcan accept acontext.Context, create the request with that context and callDo.🧰 Tools
🪛 golangci-lint (2.12.2)
[error] 101-101: net/http.Post must not be called. use net/http.NewRequestWithContext and (*net/http.Client).Do(*http.Request)
(noctx)
🤖 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 `@pkg/providers/slack/slack.go` around lines 101 - 105, Update the Slack webhook request in Provider.Send to use an http.Client with a 10-second timeout instead of the implicit http.Post client; if Send accepts a context.Context, build the request with that context and execute it via the client’s Do method, while preserving the existing error wrapping and multierr handling.Source: Linters/SAST tools
|
Rebased PR onto dev and fixed the Rocket.Chat review findings. The new workflows are waiting for fork approval; could someone approve and run them? @ehsandeep |
1 participant
Summary
rocketchatnotify provider using Rocket.Chat Incoming Webhooksalias), avatar and emojirocketchat_attachment)Example config
nuclei -u https://example.com | notify -provider rocketchatSummary by CodeRabbit