feat: cli add schema details and storage support#24
Conversation
3c51745 to
6a5157f
Compare
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughCentralizes CLI options and URL resolution, adds storage management and schema/details commands, implements a schema registry, schema-input parsing and override hooks, secure-scheme discovery, URL ID generation, many provider SchemaEntry registrations, provider target enhancements, and Python/Go parity tests. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI (Run)
participant Opts as cliOptions
participant Loader as ConfigLoader
participant Registry as SchemaRegistry
participant Storage as StorageManager
participant Notifier as NotifierDispatcher
participant Out as Stdout/Stderr
CLI->>Opts: parse flags (normalizeArgs)
CLI->>Loader: resolveNotifyURLs(opts, args)
Loader-->>CLI: taggedURL entries
CLI->>Registry: SchemaJSON()/LoadSchema() if requested
alt storage action
CLI->>Storage: RunStorage(opts, entries)
Storage->>Registry: SchemaInputsForParsed / SchemaEntryForSchema
Storage->>Storage: compute URLID, scan/prune
Storage->>Out: print listing / prune results
else notify action
CLI->>Notifier: iterate tagged entries
Notifier->>Registry: SchemaInputsFromURL -> ApplySchemaOverrides
Notifier->>Notifier: BuildRequest / Send per target
Notifier->>Out: report send results
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Poem
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/cli/cli.go`:
- Around line 1523-1528: The isStorageAction function currently uses
strings.HasPrefix with the arguments reversed which permits single-letter
matches (e.g., "s") and causes false positives; change the check in
isStorageAction to compare the lowered first arg against the "storage" prefix
correctly (e.g., call strings.HasPrefix(strings.ToLower(args[0]), "storage"))
and consider requiring a delimiter or full word (like "storage" or
"storage://"/"storage:") if you need stricter matching to avoid catching schemes
like "slack://" or "signal://".
In `@internal/cli/storage.go`:
- Around line 131-140: The resolveStoragePath function currently lets the
environment variable (defaultEnvAppriseStoragePath) override an explicitly
passed flag; change the precedence so the explicit parameter wins: trim the
explicit value first and if it is non-empty return expandPath(explicit),
otherwise check the environment variable (trimmed) and use it if present, and
finally fall back to defaultStoragePath before calling expandPath; update
references in resolveStoragePath to use these steps so --storage-path always
takes precedence over APPRISE_STORAGE_PATH.
In `@internal/notify/url_id.go`:
- Around line 41-45: Replace the fragile suffix heuristic around scheme handling
(the strings.HasSuffix(scheme, "s") branch that writes "s" or "i" to the hasher)
with an explicit registry or lookup of secure schemes: introduce a secureSchemes
map or an isSecureScheme(scheme string) bool helper and use that to decide
whether to write the secure marker ("s") or insecure marker ("i") via
hasher.Write; update the branch that references scheme and hasher.Write to
consult that registry instead of using strings.HasSuffix.
🧹 Nitpick comments (6)
internal/notify/url_id.go (1)
34-39: Path normalization edge case.When
target.Pathis empty (or whitespace), it becomes"/", thenTrimRight(fullpath, "/")reduces it back to"". This means both an empty path and a single/path will hash identically (empty string). If distinguishing between these is important for URL identity, consider preserving the/.internal/cli/details.go (1)
88-95: Redundant conditional branches.Lines 91 and 93 contain identical code; the
if idx == 0check serves no purpose here.♻️ Proposed fix
prefix := " - " for idx, template := range templates { - if idx == 0 { - fmt.Fprintf(w, "%s%s\n", prefix, template) - } else { - fmt.Fprintf(w, "%s%s\n", prefix, template) - } + _ = idx // unused in current implementation + fmt.Fprintf(w, "%s%s\n", prefix, template) }Or simply remove the conditional entirely:
prefix := " - " - for idx, template := range templates { - if idx == 0 { - fmt.Fprintf(w, "%s%s\n", prefix, template) - } else { - fmt.Fprintf(w, "%s%s\n", prefix, template) - } + for _, template := range templates { + fmt.Fprintf(w, "%s%s\n", prefix, template) }internal/cli/schema_test.go (1)
37-39: Consider improving the mismatch error message.When schemas don't match, the current message
"schema mismatch"doesn't help diagnose what differs. Consider logging or diffing the structures to aid debugging.♻️ Proposed improvement
if !reflect.DeepEqual(want, got) { - t.Fatalf("schema mismatch") + wantJSON, _ := json.MarshalIndent(want, "", " ") + gotJSON, _ := json.MarshalIndent(got, "", " ") + t.Fatalf("schema mismatch:\nwant:\n%s\n\ngot:\n%s", wantJSON, gotJSON) }For large schemas, you might prefer a diff library or truncating the output.
internal/cli/storage.go (1)
107-109: Mutation of shared options struct.Setting
opts.storagePruneDays = 0directly modifies the passed-in struct. While this works for the current flow, it could cause subtle bugs ifoptsis reused elsewhere afterRunStoragereturns.Consider using a local variable instead:
♻️ Suggested fix
+ pruneDays := opts.storagePruneDays if action == storageActionClear { - opts.storagePruneDays = 0 + pruneDays = 0 } - expiry := time.Duration(opts.storagePruneDays) * 24 * time.Hour + expiry := time.Duration(pruneDays) * 24 * time.Hourinternal/cli/config.go (1)
44-64: Missing scanner error check.After the
scanner.Scan()loop completes,scanner.Err()should be checked to detect read errors that may have occurred during scanning.♻️ Suggested fix
for scanner.Scan() { line := strings.TrimSpace(scanner.Text()) if line == "" || strings.HasPrefix(line, "#") { continue } if parsed := parseTaggedLine(line); len(parsed) > 0 { urls = append(urls, parsed...) } } + if err := scanner.Err(); err != nil { + return urls // Return partial results on error + } return urlsinternal/cli/cli.go (1)
187-201: Consider adding a TODO comment for unused options.Multiple option fields are explicitly silenced with
_ =assignments. While this avoids compiler warnings, a brief comment explaining these are placeholders for future implementation would improve clarity.♻️ Suggested fix
+ // TODO: The following options are parsed but not yet implemented _ = opts.inputFormat _ = opts.disableAsync _ = opts.attachments _ = opts.pluginPaths _ = opts.theme _ = opts.recursionDepth _ = opts.dryRun _ = opts.debug _ = opts.verbose _ = opts.interpretEscapes _ = opts.interpretEmojis _ = opts.storageMode _ = opts.storagePath _ = opts.storagePruneDays _ = opts.storageUIDLength
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/cli/cli.go`:
- Around line 59-68: The custom flag type countFlag is missing an IsBoolFlag()
bool method, so the flag package treats -v as expecting a value and consumes the
next arg; add an IsBoolFlag method on *countFlag that returns true (func (c
*countFlag) IsBoolFlag() bool { return true }) so -v is parsed as a
boolean/count flag; update the countFlag receiver methods (String, Set, and new
IsBoolFlag) to ensure consistent receiver usage and behavior when the flag is
present without an explicit value.
In `@internal/cli/storage.go`:
- Around line 285-293: The truncate function currently uses byte-length checks
and byte slicing which can split multi-byte UTF-8 characters; change it to
operate on runes: convert value to []rune (e.g., r := []rune(value)), use len(r)
for the length check, return value when len(r) <= max, handle the max <= 3 case
by returning string(r[:max]) if desired, and otherwise return string(r[:max-3])
+ "..." so truncation preserves whole Unicode characters; update the function
named truncate to use these rune-based operations.
- Around line 107-109: The code mutates the shared options struct by assigning
opts.storagePruneDays = 0 when action == storageActionClear; instead, introduce
a local variable (e.g., pruneDays := opts.storagePruneDays), set pruneDays = 0
when storageActionClear, and use that local pruneDays in the rest of RunStorage
instead of reading/writing opts.storagePruneDays; this preserves the original
opts for callers and avoids side effects while keeping the storageActionClear
behavior.
🧹 Nitpick comments (2)
internal/cli/storage.go (1)
189-223: Errors are silently ignored and the function never returns an error.
os.Removeerrors are discarded with_ = os.Remove(...), masking permission issues or locked files. Additionally, the function signature returnserrorbut always returnsnil, making error handling at the call site ineffective.♻️ Consider logging or collecting errors
-func diskPrune(path string, namespaces []string, expiry time.Duration, doDelete bool) error { +func diskPrune(path string, namespaces []string, expiry time.Duration, doDelete bool) (removed int, skipped int) { now := time.Now() ids := diskScan(path, namespaces) for _, namespace := range ids { // ... existing logic ... for _, file := range candidates { // ... existing checks ... if doDelete { - _ = os.Remove(file) + if err := os.Remove(file); err != nil { + skipped++ + continue + } + removed++ } } // ... directory cleanup ... } - return nil + return removed, skipped }internal/cli/cli.go (1)
187-201: Placeholder assignments for unimplemented features.These
_ = opts.xxxlines silence unused variable warnings. Consider adding a TODO comment to track implementation of these options.
There was a problem hiding this comment.
Actionable comments posted: 9
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
internal/notify/sfr.go (1)
3-8: Missingstrconvimport causes compilation failure.The import block is missing
strconv, butstrconv.Atoiis used at line 69. This will result in a compilation error.🐛 Proposed fix
import ( "encoding/json" "fmt" "net/url" + "strconv" "strings" )internal/notify/pushy.go (1)
3-8: Missingstrconvimport causes compilation failure.The import block is missing
strconv, butstrconv.Atoiis used at line 66. This will result in a compilation error.🐛 Proposed fix
import ( "encoding/json" "fmt" "net/url" + "strconv" "strings" )internal/notify/dot.go (1)
58-61: Fix build break: strconv is used but not imported.
strconv.Atoiis referenced inNewDotTarget, so the package won’t compile without the import.🐛 Proposed fix
import ( "encoding/json" "fmt" + "strconv" "strings" )internal/notify/burstsms.go (1)
55-59: Fix build break: strconv is used but not imported.
strconv.Atoiis referenced when parsingvalidity, so this file will not compile.🐛 Proposed fix
import ( "fmt" "net/url" + "strconv" "strings" )internal/notify/nextcloud.go (1)
52-55: Missingstrconvimport breaks the build.
strconv.Atoiis referenced but the import is absent, causing a compile error.🔧 Suggested fix
import ( "fmt" "net/url" + "strconv" "strings" )internal/notify/bark.go (1)
299-304: Re-addsortimport to fix build failure.
parseListcallssort.Strings(Line 303), but thesortpackage is no longer imported, which breaks compilation.🛠️ Proposed fix
import ( "encoding/json" "fmt" "net/url" "regexp" "strconv" + "sort" "strings" )internal/notify/sendpulse.go (1)
110-116: Missingstrconvimport breaks the build.Line 112 uses
strconv.Atoi, but the import is not present in the import block. Addstrconvto the imports.Fix import
import ( "encoding/base64" "encoding/json" "fmt" "net/http" + "strconv" "strings" )
🤖 Fix all issues with AI agents
In `@internal/notify/bulkvs.go`:
- Around line 307-315: The schema currently marks the "targets" field as
required in the map entry inside the notifier schema, but runtime allows
omission (a template omits {targets} and NewBulkVSTarget falls back to using
source when targets are not provided), causing valid runtime cases to fail
validation; fix by either changing the schema map entry for "targets" to set
"required": false so omission is allowed, or remove the runtime fallback in
NewBulkVSTarget (and update any templates that omit {targets}) so the field is
truly required—pick one approach and make the corresponding change to the
"targets" schema map or the NewBulkVSTarget function and adjust templates
accordingly.
In `@internal/notify/dingtalk.go`:
- Around line 137-190: The schema advertises a "secret" mode but
NewDingTalkTarget rejects configs with a secret; fix by aligning the schema with
implementation: remove the "secret" param entry from the params map, remove the
"secret" token from the tokens map, and eliminate any templates that include
"{secret}@" (e.g., "{schema}://{secret}@{token}/" and
"{schema}://{secret}@{token}/{targets}/"); alternatively, if you prefer to
support secrets, implement secret-handling logic inside NewDingTalkTarget to
accept and validate the secret (update parsing and NewDingTalkTarget
signature/validation to use the secret value) so the schema and runtime behavior
match.
In `@internal/notify/lametric.go`:
- Around line 253-379: Summary: The schema exposes cloud/app-token options that
NewLametricTarget does not support, causing the CLI to offer unusable cloud
modes and templates. Fix: in the schema literal used for Lametric targets (the
structure initialized alongside NewLametricTarget), restrict "mode" choice to
only "device" (remove "cloud"), remove any templates that include "{app_token}",
"{app_id}", or "{app_ver}", and delete the corresponding tokens entries
("app_token", "app_id", "app_ver") from the "tokens" map; also prune any other
template/token entries that imply cloud usage. This ensures the declared schema
matches NewLametricTarget's device-only implementation.
In `@internal/notify/matrix.go`:
- Around line 129-287: The schema currently advertises modes and target
templates that NewMatrixTarget doesn't support; update the schema to match the
t2bot-only implementation: in the "mode" field (values), remove "matrix" and
"slack" leaving only "off" and "t2bot"; prune the "templates" array to only
t2bot-compatible template patterns (remove host/user/port/targets variants that
imply room/host support); remove or mark as unsupported any token entries that
expose host/targets/room IDs (e.g., "host", "targets", "target_room_alias",
"target_room_id", "target_user") or map them to fields NewMatrixTarget actually
uses (token/password/user) so configs can't claim unsupported features; and fix
the "Matrix API Verion" string to "Matrix API Version". Ensure changes reference
NewMatrixTarget behavior so schema and implementation remain consistent.
In `@internal/notify/notificationapi.go`:
- Around line 661-668: The schema for the "targets" field must be changed to
match runtime validation: BuildRequest currently errors when len(n.targets) == 0
(refer to BuildRequest and n.targets), so update the "targets" schema entry (the
map keyed "targets") to set "required" to true instead of false so invalid
configs are rejected at validation time rather than failing later at send time.
In `@internal/notify/pushed.go`:
- Line 6: Remove the unused "strings" import from the import block in
internal/notify/pushed.go so the file compiles; locate the import declaration
near the top of the file (the import list containing "strings") and delete that
entry (or the import line) leaving only the packages actually referenced by the
code (e.g., any imports used by functions in this file).
In `@internal/notify/pushover.go`:
- Around line 91-276: The schema advertises attachment_support but
PushoverTarget has no attachment handling; update the SchemaEntry passed to
RegisterSchemaEntryOrdered for Pushover to not advertise attachments by changing
the "attachment_support" value from true to false (or remove the key) so it
accurately reflects the implementation; reference the RegisterSchemaEntryOrdered
call and the Pushover-related SchemaEntry where "attachment_support" is
currently true.
In `@internal/notify/qq.go`:
- Around line 3-7: The import list in internal/notify/qq.go includes the unused
package "strings", which causes a Go build error; remove "strings" from the
import block so only the used packages (fmt and net/url) remain, updating the
import declaration in that file accordingly (check the import block near the top
of qq.go).
In `@internal/notify/rocket.go`:
- Around line 105-302: The schema advertises token/basic auth and
username/password/token templates but NewRocketChatTarget only supports webhook
mode; update the schema registered in the init() call (the SchemaEntry passed to
RegisterSchemaEntryOrdered) to match implementation by restricting to
webhook-only: remove or set "mode" allowed values to only "webhook", remove
templates that include user/password/token, and remove/adjust tokens entries for
"user", "password", and "token" (or mark them unused) so CLI validation cannot
accept unsupported auth modes; alternatively, if you prefer to support
token/basic, implement those flows inside NewRocketChatTarget to parse and
authenticate using the "user"/"password"/"token" tokens.
🟡 Minor comments (16)
internal/notify/resend.go-281-312 (1)
281-312: Schema metadata inconsistencies need correction.The
map_tovalues in the schema definition don't affect query parsing (the code reads directly fromtarget.Queryusing the correct keys), but they are semantically incorrect and should be aligned with how the parser actually reads the values:
- Both
"from"and"name"currently map to"from_addr", which is wrong; they should map to"from"and"name"respectively"reply"should map to"reply", not"reply_to"These are metadata inconsistencies; the actual parsing works because
NewResendTargetdirectly accesses the correct query keys. However, correcting the schema to be accurate is necessary for proper documentation and tooling that relies on schema definitions.Proposed fix
"from": map[string]any{ - "map_to": "from_addr", + "map_to": "from", "name": "from", ... }, "name": map[string]any{ - "map_to": "from_addr", + "map_to": "name", "name": "From Name", ... }, "reply": map[string]any{ ... - "map_to": "reply_to", + "map_to": "reply", "name": "Reply To", ... },internal/notify/misskey.go-179-181 (1)
179-181: Self-referentialalias_ofappears redundant.The
tokenarg has"alias_of": "token", which references itself. This seems like either a copy-paste error or should reference a different field name.internal/notify/macosx.go-114-118 (1)
114-118: Minor typo in requirements details.There's a double space in "Additionally requires" on line 115.
Suggested fix
"requirements": map[string]any{ - "details": "Only works with Mac OS X 10.8 and higher. Additionally requires that /usr/local/bin/terminal-notifier is locally accessible.", + "details": "Only works with Mac OS X 10.8 and higher. Additionally requires that /usr/local/bin/terminal-notifier is locally accessible.", "packages_recommended": []any{}, "packages_required": []any{}, },internal/notify/emby.go-425-431 (1)
425-431: Schema inconsistency:usertoken marked as optional but required by implementation.The schema defines
userwith"required": false, butNewEmbyTarget(lines 37-40) validates thatuseris non-empty and returns an error if missing. This mismatch could confuse consumers relying on the schema for validation.Suggested fix
"user": map[string]any{ "map_to": "user", "name": "Username", "private": false, - "required": false, + "required": true, "type": "string", },internal/notify/fcm.go-241-248 (1)
241-248: Fix UI label for the priority option.
Thepriorityargument is labeled as “Mode”, which looks like a copy/paste typo and will surface in CLI/schema output.🛠️ Proposed fix
- "priority": map[string]any{ - "map_to": "priority", - "name": "Mode", + "priority": map[string]any{ + "map_to": "priority", + "name": "Priority", "private": false, "required": false, "type": "choice:string", "values": []string{"min", "low", "normal", "high", "max"}, },internal/notify/revolt.go-225-233 (1)
225-233: Marktargetsas required to match runtime validation.
NewRevoltTargetreturns an error when no targets are provided, but the schema marks them optional. Requiring them avoids configs that validate but fail at send time.🔧 Suggested fix
"targets": map[string]any{ "delim": []string{"/"}, "group": []string{"target_channel"}, "map_to": "targets", "name": "Targets", "private": false, - "required": false, + "required": true, "type": "list:string", },internal/notify/africas_talking.go-353-360 (1)
353-360: Require at least one target in the schema.The runtime errors when
targetsis empty, but the schema marks it as optional. Making it required avoids configs that validate but fail at send time.🔧 Suggested fix
"targets": map[string]any{ "delim": []string{"/"}, "group": []string{"target_phone"}, "map_to": "targets", "name": "Targets", "private": false, - "required": false, + "required": true, "type": "list:string", },internal/notify/dapnet.go-343-349 (1)
343-349: Aligntarget_callsignregex with runtime validation.The schema regex is looser than
dapnetCallSignRe, so invalid callsigns may pass schema validation but get dropped at runtime, producing “missing targets.” Consider matching the same pattern to avoid surprising failures.🔧 Suggested fix
- "target_callsign": map[string]any{ - "map_to": "targets", - "name": "Target Callsign", - "private": false, - "regex": []string{"^[a-z0-9]{2,5}(-[a-z0-9]{1,2})?$", "i"}, - "required": false, - "type": "string", - }, + "target_callsign": map[string]any{ + "map_to": "targets", + "name": "Target Callsign", + "private": false, + "regex": []string{"^[0-9a-z]{1,2}[0-9][a-z0-9]{1,3}(-[0-9]{1,2})?$", "i"}, + "required": false, + "type": "string", + },internal/notify/parseplatform.go-278-280 (1)
278-280: Trim the leading whitespace inservice_url.
The value starts with a space, which can surface as a broken link in schema/CLI output.🛠️ Suggested fix
- "service_url": " https://parseplatform.org/", + "service_url": "https://parseplatform.org/",internal/notify/sinch.go-217-225 (1)
217-225: Constrainregionvalues to the runtime allowlist.
The runtime only acceptsusandeu, but the schema allows any two-letter value, so schema validation can pass values that later fail.🛠️ Suggested fix
- "region": map[string]any{ - "default": "us", - "map_to": "region", - "name": "Region", - "private": false, - "regex": []string{"^[a-z]{2}$", "i"}, - "required": false, - "type": "string", - }, + "region": map[string]any{ + "default": "us", + "map_to": "region", + "name": "Region", + "private": false, + "required": false, + "type": "choice:string", + "values": []string{"eu", "us"}, + },internal/notify/notica.go-211-218 (1)
211-218: Changemap_tofrom"fullpath"to"path".The schema token
pathis mapped tofullpath, but NoticaTarget has apathfield (notfullpath), and the templates use{path}variables, not{fullpath}. This is inconsistent with the struct definition and template variables. Updatemap_toto"path"to match the actual field name and template usage across the service."path": map[string]any{ "default": "/", - "map_to": "fullpath", + "map_to": "path", "name": "Path", "private": false, "required": false, "type": "string", },internal/notify/msteams.go-308-308 (1)
308-308: Fix typo in service URL.
"micrsoft"should be"microsoft".Proposed fix
- "service_url": "https://teams.micrsoft.com/", + "service_url": "https://teams.microsoft.com/",internal/notify/opsgenie.go-587-592 (1)
587-592: Remove trailing space in "Targets " label.
Minor UI text polish to avoid extra whitespace in schema details.🧹 Suggested fix
"targets": map[string]any{ "delim": []string{"/"}, "group": []string{"target_escalation", "target_schedule", "target_team", "target_user"}, "map_to": "targets", - "name": "Targets ", + "name": "Targets", "private": false, "required": false, "type": "list:string",internal/notify/mqtt.go-123-138 (1)
123-138: Inconsistency between password requirement and templates.The
passwordtoken is marked asrequired: true(line 136), but the templates on line 123 include variants without password (e.g.,{schema}://{user}@{host}/{topic}). This inconsistency may cause validation to reject valid URLs that don't include a password.Consider changing
requiredtofalseif password-less authentication is supported:Suggested fix
"password": map[string]any{ "map_to": "password", "name": "Password", "private": true, - "required": true, + "required": false, "type": "string", },internal/notify/guilded.go-148-155 (1)
148-155: Copy-paste artifact: "Discord Flags" in Guilded schema.The
flagsargument hasname: "Discord Flags"but this schema is for the Guilded service. This appears to be a copy-paste oversight from a Discord schema definition.Suggested fix
"flags": map[string]any{ "map_to": "flags", "min": 0, - "name": "Discord Flags", + "name": "Guilded Flags", "private": false, "required": false, "type": "int", },internal/notify/schema_slack.go-136-143 (1)
136-143: Potential typo in template on line 138.The template
"{schema}://{botname}@{token_a}/{token_b}{token_c}"is missing a/between{token_b}and{token_c}, while line 137 uses{token_a}/{token_b}/{token_c}with proper separators. This appears inconsistent.Proposed fix
"templates": []string{ "{schema}://{token_a}/{token_b}/{token_c}", - "{schema}://{botname}@{token_a}/{token_b}{token_c}", + "{schema}://{botname}@{token_a}/{token_b}/{token_c}", "{schema}://{token_a}/{token_b}/{token_c}/{targets}", "{schema}://{botname}@{token_a}/{token_b}/{token_c}/{targets}", "{schema}://{access_token}/", "{schema}://{access_token}/{targets}", },
🧹 Nitpick comments (11)
internal/notify/popcorn.go (1)
281-288: Consider populatingprotocolsto avoid schema omission.If schema consumers rely on
protocols(not justsecure_protocols) to list supported schemes,nilmay hide PopcornNotify. If that’s the case, set it explicitly; otherwise confirmnilis intended.Proposed tweak (if protocols list is required)
- "protocols": nil, + "protocols": []string{"popcorn"},internal/notify/misskey.go (1)
235-241: Consider marking the access token as private.The
tokenfield contains a Misskey access token, which is sensitive authentication data. Marking it as"private": truewould be more consistent with security best practices and prevent accidental exposure in logs or UI.Suggested fix
"token": map[string]any{ "map_to": "token", "name": "Access Token", - "private": false, + "private": true, "required": true, "type": "string", },internal/notify/schema_override_registry_test.go (1)
5-19: Consider test isolation for global registry modifications.The test registers an override globally but doesn't clean up afterward. If the override registry supports removal or reset, consider cleaning up to prevent potential interference with other tests in the same process.
If cleanup isn't supported, consider using a unique schema name per test run (e.g., with
t.Name()) to avoid collisions.internal/notify/smseagle.go (1)
432-438: Drop redundant self-alias fortoken.
"token"aliasing itself doesn’t add behavior and may confuse schema consumers.♻️ Suggested cleanup
- "token": map[string]any{ - "alias_of": "token", - },internal/notify/schema_override_registry.go (1)
23-55: Guard against a nilvaluesmap in overrides.
If an override writes intovaluesand the caller passednil, it will panic. A defensive init keeps the API safer.🛡️ Defensive guard
func ApplySchemaOverrides(schema string, target *ParsedURL, values map[string]SchemaValue) { - schemaOverrides.apply(schema, target, values) + if values == nil { + values = map[string]SchemaValue{} + } + schemaOverrides.apply(schema, target, values) }internal/notify/azure.go (2)
27-34: Inconsistent default value types for float fields.The
ctoandrtofields are declared with"type": "float"but have integer default values (4instead of4.0). Other schema files (e.g.,schema_json.go,schema_ntfy.go) use4.0for consistency. This could cause type assertion issues if the schema processing code expectsfloat64.Suggested fix for consistency
"cto": map[string]any{ - "default": 4, + "default": 4.0, "map_to": "cto", "name": "Socket Connect Timeout", "private": false, "required": false, "type": "float", },"rto": map[string]any{ - "default": 4, + "default": 4.0, "map_to": "rto", "name": "Socket Read Timeout", "private": false, "required": false, "type": "float", },Also applies to: 67-74
11-11: Inconsistent slice types across schema files.The
groupfield uses[]any{}here but other schemas use[]string{}(e.g., Discord'spingfield). Similarly,packages_recommendedandpackages_requireduse[]any{}here but[]string{}in other schema files. Consider standardizing on one type for consistency across all schema registrations.Also applies to: 20-20, 167-168
internal/notify/qt.go (1)
9-16: Same inconsistency: integer defaults for float-typed fields.Same issue as in
azure.go-ctoandrtohave integer defaults (4) but are typed as"float". Consider using4.0for consistency with other schema files.Also applies to: 54-61
internal/notify/schema_inputs.go (2)
425-445: Consider caching compiled regex for frequently used delimiters.
regexp.MustCompileis called on every invocation ofsplitByDelims. If this function is called frequently with the same delimiter patterns, the repeated compilation could become a performance concern.Proposed optimization using a cache
+var delimRegexCache sync.Map // map[string]*regexp.Regexp + func splitByDelims(raw string, delims []string) []string { trimmed := strings.TrimSpace(raw) if trimmed == "" { return nil } if len(delims) == 0 { return []string{trimmed} } pattern := buildDelimRegex(delims) - re := regexp.MustCompile(pattern) + var re *regexp.Regexp + if cached, ok := delimRegexCache.Load(pattern); ok { + re = cached.(*regexp.Regexp) + } else { + re = regexp.MustCompile(pattern) + delimRegexCache.Store(pattern, re) + } parts := re.Split(trimmed, -1) values := make([]string, 0, len(parts)) for _, part := range parts {
638-662: Redundant bounds check at line 640.The condition
idx > len(segments)at line 640 can never be true. The variableidxstarts at 0 and is only incremented after successful segment matches, where bounds are already checked (lines 647, 657, 665). This check is unreachable.Proposed simplification
idx := 0 for i, pattern := range patternSegments { - if idx > len(segments) { - return false - } if len(pattern) == 1 && pattern[0].isToken {internal/notify/schema_registry.go (1)
20-31: Variable name shadows type name.The global variable
schemaRegistry(line 27) shadows the type nameschemaRegistry(line 20). While valid Go, this can cause confusion when reading the code.Consider renaming for clarity
-type schemaRegistry struct { +type schemaRegistryStore struct { mu sync.RWMutex entries []schemaEntryRecord asset map[string]any bySchema map[string]SchemaEntry } -var schemaRegistry = &schemaRegistry{ +var schemaRegistry = &schemaRegistryStore{ entries: []schemaEntryRecord{}, asset: map[string]any{}, bySchema: map[string]SchemaEntry{}, }
| "targets": map[string]any{ | ||
| "delim": []string{"/"}, | ||
| "group": []string{"target_phone"}, | ||
| "map_to": "targets", | ||
| "name": "Targets", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "list:string", | ||
| }, |
There was a problem hiding this comment.
Schema marks targets required, but templates/runtime allow omission.
Line 272 includes a template without {targets}, and NewBulkVSTarget defaults targets to source when none are provided (Line 67-69). With required: true, schema-driven validation will reject otherwise valid URLs. Consider making it optional (or remove the fallback/template if you intend to enforce it).
🛠️ Suggested fix
"targets": map[string]any{
"delim": []string{"/"},
"group": []string{"target_phone"},
"map_to": "targets",
"name": "Targets",
"private": false,
- "required": true,
+ "required": false,
"type": "list:string",
},📝 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.
| "targets": map[string]any{ | |
| "delim": []string{"/"}, | |
| "group": []string{"target_phone"}, | |
| "map_to": "targets", | |
| "name": "Targets", | |
| "private": false, | |
| "required": true, | |
| "type": "list:string", | |
| }, | |
| "targets": map[string]any{ | |
| "delim": []string{"/"}, | |
| "group": []string{"target_phone"}, | |
| "map_to": "targets", | |
| "name": "Targets", | |
| "private": false, | |
| "required": false, | |
| "type": "list:string", | |
| }, |
🤖 Prompt for AI Agents
In `@internal/notify/bulkvs.go` around lines 307 - 315, The schema currently marks
the "targets" field as required in the map entry inside the notifier schema, but
runtime allows omission (a template omits {targets} and NewBulkVSTarget falls
back to using source when targets are not provided), causing valid runtime cases
to fail validation; fix by either changing the schema map entry for "targets" to
set "required": false so omission is allowed, or remove the runtime fallback in
NewBulkVSTarget (and update any templates that omit {targets}) so the field is
truly required—pick one approach and make the corresponding change to the
"targets" schema map or the NewBulkVSTarget function and adjust templates
accordingly.
| "mode": map[string]any{ | ||
| "default": "off", | ||
| "map_to": "mode", | ||
| "name": "Webhook Mode", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"off", "matrix", "slack", "t2bot"}, | ||
| }, | ||
| "msgtype": map[string]any{ | ||
| "default": "text", | ||
| "map_to": "msgtype", | ||
| "name": "Message Type", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"text", "notice"}, | ||
| }, | ||
| "overflow": map[string]any{ | ||
| "default": "upstream", | ||
| "map_to": "overflow", | ||
| "name": "Overflow Mode", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"split", "truncate", "upstream"}, | ||
| }, | ||
| "rto": map[string]any{ | ||
| "default": 4, | ||
| "map_to": "rto", | ||
| "name": "Socket Read Timeout", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "float", | ||
| }, | ||
| "store": map[string]any{ | ||
| "default": true, | ||
| "map_to": "store", | ||
| "name": "Persistent Storage", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| "to": map[string]any{ | ||
| "alias_of": "targets", | ||
| "delim": []string{",", " "}, | ||
| }, | ||
| "token": map[string]any{ | ||
| "alias_of": "token", | ||
| }, | ||
| "tz": map[string]any{ | ||
| "default": nil, | ||
| "map_to": "tz", | ||
| "name": "Timezone", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "verify": map[string]any{ | ||
| "default": true, | ||
| "map_to": "verify", | ||
| "name": "Verify SSL", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| "version": map[string]any{ | ||
| "default": "3", | ||
| "map_to": "version", | ||
| "name": "Matrix API Verion", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"2", "3"}, | ||
| }, | ||
| }, | ||
| "kwargs": map[string]any{}, | ||
| "templates": []string{"{schema}://{token}", "{schema}://{user}@{token}", "{schema}://{user}:{password}@{host}/{targets}", "{schema}://{user}:{password}@{host}:{port}/{targets}", "{schema}://{token}@{host}/{targets}", "{schema}://{token}@{host}:{port}/{targets}", "{schema}://{user}:{token}@{host}/{targets}", "{schema}://{user}:{token}@{host}:{port}/{targets}"}, | ||
| "tokens": map[string]any{ | ||
| "host": map[string]any{ | ||
| "map_to": "host", | ||
| "name": "Hostname", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "password": map[string]any{ | ||
| "map_to": "password", | ||
| "name": "Password", | ||
| "private": true, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "port": map[string]any{ | ||
| "map_to": "port", | ||
| "max": 65535, | ||
| "min": 1, | ||
| "name": "Port", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "int", | ||
| }, | ||
| "schema": map[string]any{ | ||
| "map_to": "schema", | ||
| "name": "Schema", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "choice:string", | ||
| "values": []string{"matrix", "matrixs"}, | ||
| }, | ||
| "target_room_alias": map[string]any{ | ||
| "map_to": "targets", | ||
| "name": "Target Room Alias", | ||
| "prefix": "#", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "target_room_id": map[string]any{ | ||
| "map_to": "targets", | ||
| "name": "Target Room ID", | ||
| "prefix": "!", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "target_user": map[string]any{ | ||
| "map_to": "targets", | ||
| "name": "Target User", | ||
| "prefix": "@", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "targets": map[string]any{ | ||
| "delim": []string{"/"}, | ||
| "group": []string{"target_room_alias", "target_room_id", "target_user"}, | ||
| "map_to": "targets", | ||
| "name": "Targets", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "list:string", | ||
| }, | ||
| "token": map[string]any{ | ||
| "map_to": "password", | ||
| "name": "Access Token", | ||
| "private": true, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "user": map[string]any{ | ||
| "map_to": "user", | ||
| "name": "Username", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Matrix schema lists unsupported modes/targets.
NewMatrixTarget only accepts t2bot and ignores room targets, but the schema allows matrix/slack modes and host/targets templates. This will admit configs that fail at runtime. Align the schema with current support (or implement full Matrix support). Also fix the “Matrix API Verion” typo.
🛠️ Suggested fix (t2bot-only alignment)
@@
- "mode": map[string]any{
- "default": "off",
+ "mode": map[string]any{
+ "default": "t2bot",
@@
- "values": []string{"off", "matrix", "slack", "t2bot"},
+ "values": []string{"t2bot"},
},
@@
- "templates": []string{"{schema}://{token}", "{schema}://{user}@{token}", "{schema}://{user}:{password}@{host}/{targets}", "{schema}://{user}:{password}@{host}:{port}/{targets}", "{schema}://{token}@{host}/{targets}", "{schema}://{token}@{host}:{port}/{targets}", "{schema}://{user}:{token}@{host}/{targets}", "{schema}://{user}:{token}@{host}:{port}/{targets}"},
+ "templates": []string{"{schema}://{token}"},
@@
- "name": "Matrix API Verion",
+ "name": "Matrix API Version",🤖 Prompt for AI Agents
In `@internal/notify/matrix.go` around lines 129 - 287, The schema currently
advertises modes and target templates that NewMatrixTarget doesn't support;
update the schema to match the t2bot-only implementation: in the "mode" field
(values), remove "matrix" and "slack" leaving only "off" and "t2bot"; prune the
"templates" array to only t2bot-compatible template patterns (remove
host/user/port/targets variants that imply room/host support); remove or mark as
unsupported any token entries that expose host/targets/room IDs (e.g., "host",
"targets", "target_room_alias", "target_room_id", "target_user") or map them to
fields NewMatrixTarget actually uses (token/password/user) so configs can't
claim unsupported features; and fix the "Matrix API Verion" string to "Matrix
API Version". Ensure changes reference NewMatrixTarget behavior so schema and
implementation remain consistent.
| "targets": map[string]any{ | ||
| "delim": []string{"/"}, | ||
| "group": []string{"target_email", "target_id", "target_sms"}, | ||
| "map_to": "targets", | ||
| "name": "Targets", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "list:string", |
There was a problem hiding this comment.
Require targets in schema to match runtime validation.
BuildRequest errors when len(n.targets) == 0 (Line 206-208), but the schema marks targets as optional (Line 667). This lets invalid configs pass validation and fail later at send time.
🛠️ Suggested fix
"targets": map[string]any{
"delim": []string{"/"},
"group": []string{"target_email", "target_id", "target_sms"},
"map_to": "targets",
"name": "Targets",
"private": false,
- "required": false,
+ "required": true,
"type": "list:string",
},📝 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.
| "targets": map[string]any{ | |
| "delim": []string{"/"}, | |
| "group": []string{"target_email", "target_id", "target_sms"}, | |
| "map_to": "targets", | |
| "name": "Targets", | |
| "private": false, | |
| "required": false, | |
| "type": "list:string", | |
| "targets": map[string]any{ | |
| "delim": []string{"/"}, | |
| "group": []string{"target_email", "target_id", "target_sms"}, | |
| "map_to": "targets", | |
| "name": "Targets", | |
| "private": false, | |
| "required": true, | |
| "type": "list:string", |
🤖 Prompt for AI Agents
In `@internal/notify/notificationapi.go` around lines 661 - 668, The schema for
the "targets" field must be changed to match runtime validation: BuildRequest
currently errors when len(n.targets) == 0 (refer to BuildRequest and n.targets),
so update the "targets" schema entry (the map keyed "targets") to set "required"
to true instead of false so invalid configs are rejected at validation time
rather than failing later at send time.
| func init() { | ||
| RegisterSchemaEntryOrdered(35, SchemaEntry{ | ||
| "attachment_support": true, | ||
| "category": "native", | ||
| "details": map[string]any{ | ||
| "args": map[string]any{ | ||
| "cto": map[string]any{ | ||
| "default": 4, | ||
| "map_to": "cto", | ||
| "name": "Socket Connect Timeout", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "float", | ||
| }, | ||
| "emojis": map[string]any{ | ||
| "default": false, | ||
| "map_to": "emojis", | ||
| "name": "Interpret Emojis", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| "expire": map[string]any{ | ||
| "default": 3600, | ||
| "map_to": "expire", | ||
| "max": 10800, | ||
| "min": 0, | ||
| "name": "Expire", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "int", | ||
| }, | ||
| "format": map[string]any{ | ||
| "default": "text", | ||
| "map_to": "format", | ||
| "name": "Notify Format", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"html", "markdown", "text"}, | ||
| }, | ||
| "overflow": map[string]any{ | ||
| "default": "upstream", | ||
| "map_to": "overflow", | ||
| "name": "Overflow Mode", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"split", "truncate", "upstream"}, | ||
| }, | ||
| "priority": map[string]any{ | ||
| "default": 0, | ||
| "map_to": "priority", | ||
| "name": "Priority", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:int", | ||
| "values": []any{-2, -1, 0, 1, 2}, | ||
| }, | ||
| "retry": map[string]any{ | ||
| "default": 900, | ||
| "map_to": "retry", | ||
| "min": 30, | ||
| "name": "Retry", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "int", | ||
| }, | ||
| "rto": map[string]any{ | ||
| "default": 4, | ||
| "map_to": "rto", | ||
| "name": "Socket Read Timeout", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "float", | ||
| }, | ||
| "sound": map[string]any{ | ||
| "default": "pushover", | ||
| "map_to": "sound", | ||
| "name": "Sound", | ||
| "private": false, | ||
| "regex": []string{"^[a-z]{1,12}$", "i"}, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "store": map[string]any{ | ||
| "default": true, | ||
| "map_to": "store", | ||
| "name": "Persistent Storage", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| "to": map[string]any{ | ||
| "alias_of": "targets", | ||
| "delim": []string{",", " "}, | ||
| }, | ||
| "tz": map[string]any{ | ||
| "default": nil, | ||
| "map_to": "tz", | ||
| "name": "Timezone", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "url": map[string]any{ | ||
| "map_to": "supplemental_url", | ||
| "name": "URL", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "url_title": map[string]any{ | ||
| "map_to": "supplemental_url_title", | ||
| "name": "URL Title", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "verify": map[string]any{ | ||
| "default": true, | ||
| "map_to": "verify", | ||
| "name": "Verify SSL", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| }, | ||
| "kwargs": map[string]any{}, | ||
| "templates": []string{"{schema}://{user_key}@{token}", "{schema}://{user_key}@{token}/{targets}"}, | ||
| "tokens": map[string]any{ | ||
| "schema": map[string]any{ | ||
| "default": "pover", | ||
| "map_to": "schema", | ||
| "name": "Schema", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "choice:string", | ||
| "values": []string{"pover"}, | ||
| }, | ||
| "target_device": map[string]any{ | ||
| "map_to": "targets", | ||
| "name": "Target Device", | ||
| "private": false, | ||
| "regex": []string{"^[a-z0-9_-]{1,25}$", "i"}, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "targets": map[string]any{ | ||
| "delim": []string{"/"}, | ||
| "group": []string{"target_device"}, | ||
| "map_to": "targets", | ||
| "name": "Targets", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "list:string", | ||
| }, | ||
| "token": map[string]any{ | ||
| "map_to": "token", | ||
| "name": "Access Token", | ||
| "private": true, | ||
| "required": true, | ||
| "type": "string", | ||
| }, | ||
| "user_key": map[string]any{ | ||
| "map_to": "user_key", | ||
| "name": "User Key", | ||
| "private": true, | ||
| "required": true, | ||
| "type": "string", | ||
| }, | ||
| }, | ||
| }, | ||
| "enabled": true, | ||
| "protocols": nil, | ||
| "requirements": map[string]any{ | ||
| "details": "", | ||
| "packages_recommended": []any{}, | ||
| "packages_required": []any{}, | ||
| }, | ||
| "secure_protocols": []string{"pover"}, | ||
| "service_name": "Pushover", | ||
| "service_url": "https://pushover.net/", | ||
| "setup_url": "https://appriseit.com/services/pushover/", | ||
| }) | ||
| } |
There was a problem hiding this comment.
attachment_support is enabled but no attachment handling exists.
The current PushoverTarget request builder only sends form-encoded fields and has no attachment path, so advertising attachment support will mislead CLI/config users. Consider disabling until attachments are implemented.
🛠️ Proposed fix
- "attachment_support": true,
+ "attachment_support": false,📝 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.
| func init() { | |
| RegisterSchemaEntryOrdered(35, SchemaEntry{ | |
| "attachment_support": true, | |
| "category": "native", | |
| "details": map[string]any{ | |
| "args": map[string]any{ | |
| "cto": map[string]any{ | |
| "default": 4, | |
| "map_to": "cto", | |
| "name": "Socket Connect Timeout", | |
| "private": false, | |
| "required": false, | |
| "type": "float", | |
| }, | |
| "emojis": map[string]any{ | |
| "default": false, | |
| "map_to": "emojis", | |
| "name": "Interpret Emojis", | |
| "private": false, | |
| "required": false, | |
| "type": "bool", | |
| }, | |
| "expire": map[string]any{ | |
| "default": 3600, | |
| "map_to": "expire", | |
| "max": 10800, | |
| "min": 0, | |
| "name": "Expire", | |
| "private": false, | |
| "required": false, | |
| "type": "int", | |
| }, | |
| "format": map[string]any{ | |
| "default": "text", | |
| "map_to": "format", | |
| "name": "Notify Format", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"html", "markdown", "text"}, | |
| }, | |
| "overflow": map[string]any{ | |
| "default": "upstream", | |
| "map_to": "overflow", | |
| "name": "Overflow Mode", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"split", "truncate", "upstream"}, | |
| }, | |
| "priority": map[string]any{ | |
| "default": 0, | |
| "map_to": "priority", | |
| "name": "Priority", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:int", | |
| "values": []any{-2, -1, 0, 1, 2}, | |
| }, | |
| "retry": map[string]any{ | |
| "default": 900, | |
| "map_to": "retry", | |
| "min": 30, | |
| "name": "Retry", | |
| "private": false, | |
| "required": false, | |
| "type": "int", | |
| }, | |
| "rto": map[string]any{ | |
| "default": 4, | |
| "map_to": "rto", | |
| "name": "Socket Read Timeout", | |
| "private": false, | |
| "required": false, | |
| "type": "float", | |
| }, | |
| "sound": map[string]any{ | |
| "default": "pushover", | |
| "map_to": "sound", | |
| "name": "Sound", | |
| "private": false, | |
| "regex": []string{"^[a-z]{1,12}$", "i"}, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "store": map[string]any{ | |
| "default": true, | |
| "map_to": "store", | |
| "name": "Persistent Storage", | |
| "private": false, | |
| "required": false, | |
| "type": "bool", | |
| }, | |
| "to": map[string]any{ | |
| "alias_of": "targets", | |
| "delim": []string{",", " "}, | |
| }, | |
| "tz": map[string]any{ | |
| "default": nil, | |
| "map_to": "tz", | |
| "name": "Timezone", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "url": map[string]any{ | |
| "map_to": "supplemental_url", | |
| "name": "URL", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "url_title": map[string]any{ | |
| "map_to": "supplemental_url_title", | |
| "name": "URL Title", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "verify": map[string]any{ | |
| "default": true, | |
| "map_to": "verify", | |
| "name": "Verify SSL", | |
| "private": false, | |
| "required": false, | |
| "type": "bool", | |
| }, | |
| }, | |
| "kwargs": map[string]any{}, | |
| "templates": []string{"{schema}://{user_key}@{token}", "{schema}://{user_key}@{token}/{targets}"}, | |
| "tokens": map[string]any{ | |
| "schema": map[string]any{ | |
| "default": "pover", | |
| "map_to": "schema", | |
| "name": "Schema", | |
| "private": false, | |
| "required": true, | |
| "type": "choice:string", | |
| "values": []string{"pover"}, | |
| }, | |
| "target_device": map[string]any{ | |
| "map_to": "targets", | |
| "name": "Target Device", | |
| "private": false, | |
| "regex": []string{"^[a-z0-9_-]{1,25}$", "i"}, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "targets": map[string]any{ | |
| "delim": []string{"/"}, | |
| "group": []string{"target_device"}, | |
| "map_to": "targets", | |
| "name": "Targets", | |
| "private": false, | |
| "required": false, | |
| "type": "list:string", | |
| }, | |
| "token": map[string]any{ | |
| "map_to": "token", | |
| "name": "Access Token", | |
| "private": true, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| "user_key": map[string]any{ | |
| "map_to": "user_key", | |
| "name": "User Key", | |
| "private": true, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| }, | |
| }, | |
| "enabled": true, | |
| "protocols": nil, | |
| "requirements": map[string]any{ | |
| "details": "", | |
| "packages_recommended": []any{}, | |
| "packages_required": []any{}, | |
| }, | |
| "secure_protocols": []string{"pover"}, | |
| "service_name": "Pushover", | |
| "service_url": "https://pushover.net/", | |
| "setup_url": "https://appriseit.com/services/pushover/", | |
| }) | |
| } | |
| func init() { | |
| RegisterSchemaEntryOrdered(35, SchemaEntry{ | |
| "attachment_support": false, | |
| "category": "native", | |
| "details": map[string]any{ | |
| "args": map[string]any{ | |
| "cto": map[string]any{ | |
| "default": 4, | |
| "map_to": "cto", | |
| "name": "Socket Connect Timeout", | |
| "private": false, | |
| "required": false, | |
| "type": "float", | |
| }, | |
| "emojis": map[string]any{ | |
| "default": false, | |
| "map_to": "emojis", | |
| "name": "Interpret Emojis", | |
| "private": false, | |
| "required": false, | |
| "type": "bool", | |
| }, | |
| "expire": map[string]any{ | |
| "default": 3600, | |
| "map_to": "expire", | |
| "max": 10800, | |
| "min": 0, | |
| "name": "Expire", | |
| "private": false, | |
| "required": false, | |
| "type": "int", | |
| }, | |
| "format": map[string]any{ | |
| "default": "text", | |
| "map_to": "format", | |
| "name": "Notify Format", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"html", "markdown", "text"}, | |
| }, | |
| "overflow": map[string]any{ | |
| "default": "upstream", | |
| "map_to": "overflow", | |
| "name": "Overflow Mode", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"split", "truncate", "upstream"}, | |
| }, | |
| "priority": map[string]any{ | |
| "default": 0, | |
| "map_to": "priority", | |
| "name": "Priority", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:int", | |
| "values": []any{-2, -1, 0, 1, 2}, | |
| }, | |
| "retry": map[string]any{ | |
| "default": 900, | |
| "map_to": "retry", | |
| "min": 30, | |
| "name": "Retry", | |
| "private": false, | |
| "required": false, | |
| "type": "int", | |
| }, | |
| "rto": map[string]any{ | |
| "default": 4, | |
| "map_to": "rto", | |
| "name": "Socket Read Timeout", | |
| "private": false, | |
| "required": false, | |
| "type": "float", | |
| }, | |
| "sound": map[string]any{ | |
| "default": "pushover", | |
| "map_to": "sound", | |
| "name": "Sound", | |
| "private": false, | |
| "regex": []string{"^[a-z]{1,12}$", "i"}, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "store": map[string]any{ | |
| "default": true, | |
| "map_to": "store", | |
| "name": "Persistent Storage", | |
| "private": false, | |
| "required": false, | |
| "type": "bool", | |
| }, | |
| "to": map[string]any{ | |
| "alias_of": "targets", | |
| "delim": []string{",", " "}, | |
| }, | |
| "tz": map[string]any{ | |
| "default": nil, | |
| "map_to": "tz", | |
| "name": "Timezone", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "url": map[string]any{ | |
| "map_to": "supplemental_url", | |
| "name": "URL", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "url_title": map[string]any{ | |
| "map_to": "supplemental_url_title", | |
| "name": "URL Title", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "verify": map[string]any{ | |
| "default": true, | |
| "map_to": "verify", | |
| "name": "Verify SSL", | |
| "private": false, | |
| "required": false, | |
| "type": "bool", | |
| }, | |
| }, | |
| "kwargs": map[string]any{}, | |
| "templates": []string{"{schema}://{user_key}@{token}", "{schema}://{user_key}@{token}/{targets}"}, | |
| "tokens": map[string]any{ | |
| "schema": map[string]any{ | |
| "default": "pover", | |
| "map_to": "schema", | |
| "name": "Schema", | |
| "private": false, | |
| "required": true, | |
| "type": "choice:string", | |
| "values": []string{"pover"}, | |
| }, | |
| "target_device": map[string]any{ | |
| "map_to": "targets", | |
| "name": "Target Device", | |
| "private": false, | |
| "regex": []string{"^[a-z0-9_-]{1,25}$", "i"}, | |
| "required": false, | |
| "type": "string", | |
| }, | |
| "targets": map[string]any{ | |
| "delim": []string{"/"}, | |
| "group": []string{"target_device"}, | |
| "map_to": "targets", | |
| "name": "Targets", | |
| "private": false, | |
| "required": false, | |
| "type": "list:string", | |
| }, | |
| "token": map[string]any{ | |
| "map_to": "token", | |
| "name": "Access Token", | |
| "private": true, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| "user_key": map[string]any{ | |
| "map_to": "user_key", | |
| "name": "User Key", | |
| "private": true, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| }, | |
| }, | |
| "enabled": true, | |
| "protocols": nil, | |
| "requirements": map[string]any{ | |
| "details": "", | |
| "packages_recommended": []any{}, | |
| "packages_required": []any{}, | |
| }, | |
| "secure_protocols": []string{"pover"}, | |
| "service_name": "Pushover", | |
| "service_url": "https://pushover.net/", | |
| "setup_url": "https://appriseit.com/services/pushover/", | |
| }) | |
| } |
🤖 Prompt for AI Agents
In `@internal/notify/pushover.go` around lines 91 - 276, The schema advertises
attachment_support but PushoverTarget has no attachment handling; update the
SchemaEntry passed to RegisterSchemaEntryOrdered for Pushover to not advertise
attachments by changing the "attachment_support" value from true to false (or
remove the key) so it accurately reflects the implementation; reference the
RegisterSchemaEntryOrdered call and the Pushover-related SchemaEntry where
"attachment_support" is currently true.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/notify/pushed.go`:
- Around line 92-217: The schema advertises target routing that isn't
implemented: NewPushedTarget does not parse URL targets and BuildRequest always
sends target_type:"app", so remove the unsupported target fields instead of
leaving them misleading. Remove the "target_channel", "target_user", and
"targets" token entries, remove the "to" arg alias (or its delim/grouping), and
remove any templates that include "{targets}" so the schema matches the actual
behavior of NewPushedTarget and BuildRequest; alternatively, if you prefer to
keep targets, implement parsing in NewPushedTarget and propagate parsed targets
into BuildRequest payloads instead (choose one approach and make the
corresponding changes consistently).
In `@internal/notify/sendpulse.go`:
- Around line 423-424: The first example template in the templates array is
missing the required {client_id} token which is referenced by the tokens map and
used by NewSendPulseTarget; update the first template string to include
{client_id} (e.g., "{schema}://{user}@{host}/{client_id}/{client_secret}/") so
it matches the tokens map and NewSendPulseTarget expectations and prevents
generation of invalid example URLs.
In `@internal/notify/sfr.go`:
- Around line 280-321: The schema entry for the "sender" field incorrectly sets
"required": true; update the "sender" map (the sender schema in sfr.go) to mark
it optional by setting "required": false (keep the default "" if desired) so it
matches the SFR MessagesUnitairesWS/addSingleCall API where from/sender is
optional; ensure only the "required" boolean is changed and no other fields
(name, map_to, type) are modified.
🧹 Nitpick comments (4)
internal/notify/schema_details_parity_test.go (2)
64-86: Consider handling duplicateservice_nameentries.If multiple schema entries have the same
service_name, the later entry silently overwrites the earlier one in the map. This could mask parity issues if duplicates exist in one source but not the other.♻️ Suggested fix to detect duplicates
func normalizeSchemaEntries(t *testing.T, raw any) map[string]any { t.Helper() entries, ok := raw.([]any) if !ok { t.Fatalf("schema entries type mismatch: %T", raw) } out := map[string]any{} for _, item := range entries { entry, ok := item.(map[string]any) if !ok { t.Fatalf("schema entry type mismatch: %T", item) } name, ok := entry["service_name"].(string) if !ok || name == "" { t.Fatalf("schema entry missing service_name") } + if _, exists := out[name]; exists { + t.Fatalf("duplicate service_name: %s", name) + } out[name] = entry } return out }
88-97: Test output could be overwhelming for large schema mismatches.When schemas differ, the test dumps both full JSON blobs which can be thousands of lines. Consider using a diff library or limiting output to show only the differing keys/paths for easier debugging.
internal/cli/cli.go (1)
192-206: Consider tracking unimplemented features.Multiple options are explicitly silenced with blank identifier assignments, indicating incomplete feature implementation. Consider adding TODO comments or tracking these in an issue to ensure they're addressed before release.
internal/notify/schema_registry.go (1)
62-78: Duplicate schema registration silently overwrites.If the same protocol is registered by multiple entries (e.g., two init() functions registering overlapping protocols), the
bySchemamap will silently use the last registration. Consider logging a warning or returning an error if this is unintentional.
| "templates": []string{"{schema}://{user}@{host}/{client_secret}/", "{schema}://{user}@{host}/{client_id}/{client_secret}/{targets}"}, | ||
| "tokens": map[string]any{ |
There was a problem hiding this comment.
Template missing required client_id token.
client_id is required in tokens and by NewSendPulseTarget, but the first template omits it, which can generate an invalid example URL.
🛠️ Suggested fix
- "templates": []string{"{schema}://{user}@{host}/{client_secret}/", "{schema}://{user}@{host}/{client_id}/{client_secret}/{targets}"},
+ "templates": []string{
+ "{schema}://{user}@{host}/{client_id}/{client_secret}",
+ "{schema}://{user}@{host}/{client_id}/{client_secret}/{targets}",
+ },📝 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.
| "templates": []string{"{schema}://{user}@{host}/{client_secret}/", "{schema}://{user}@{host}/{client_id}/{client_secret}/{targets}"}, | |
| "tokens": map[string]any{ | |
| "templates": []string{ | |
| "{schema}://{user}@{host}/{client_id}/{client_secret}", | |
| "{schema}://{user}@{host}/{client_id}/{client_secret}/{targets}", | |
| }, | |
| "tokens": map[string]any{ |
🤖 Prompt for AI Agents
In `@internal/notify/sendpulse.go` around lines 423 - 424, The first example
template in the templates array is missing the required {client_id} token which
is referenced by the tokens map and used by NewSendPulseTarget; update the first
template string to include {client_id} (e.g.,
"{schema}://{user}@{host}/{client_id}/{client_secret}/") so it matches the
tokens map and NewSendPulseTarget expectations and prevents generation of
invalid example URLs.
| "lang": map[string]any{ | ||
| "default": "fr_FR", | ||
| "map_to": "lang", | ||
| "name": "Language", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "string", | ||
| }, | ||
| "media": map[string]any{ | ||
| "default": "SMSUnicode", | ||
| "map_to": "media", | ||
| "name": "Media Type", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "string", | ||
| "values": []string{"SMS", "SMSLong", "SMSUnicode", "SMSUnicodeLong"}, | ||
| }, | ||
| "overflow": map[string]any{ | ||
| "default": "upstream", | ||
| "map_to": "overflow", | ||
| "name": "Overflow Mode", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"split", "truncate", "upstream"}, | ||
| }, | ||
| "rto": map[string]any{ | ||
| "default": 4, | ||
| "map_to": "rto", | ||
| "name": "Socket Read Timeout", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "float", | ||
| }, | ||
| "sender": map[string]any{ | ||
| "default": "", | ||
| "map_to": "sender", | ||
| "name": "Sender Name", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "string", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n internal/notify/sfr.go | head -100Repository: unraid/apprise-go
Length of output: 2956
🏁 Script executed:
wc -l internal/notify/sfr.goRepository: unraid/apprise-go
Length of output: 87
🏁 Script executed:
sed -n '1,50p' internal/notify/sfr.goRepository: unraid/apprise-go
Length of output: 1182
🏁 Script executed:
# Find NewSFRTarget function
rg "func NewSFRTarget" -A 20Repository: unraid/apprise-go
Length of output: 1083
🏁 Script executed:
# Find schema registration for SFR
rg "\"sfr\"" -B 5 -A 30 internal/notify/sfr.go | head -150Repository: unraid/apprise-go
Length of output: 1591
🌐 Web query:
SFR MessagesUnitairesWS addSingleCall API required parameters documentation
💡 Result:
For SFR DMC endpoint MessagesUnitairesWS/addSingleCall, the request contains two JSON parameters: authenticate and messageUnitaire.
Required parameters
1) authenticate (required)
serviceId(required)servicePassword(required)spaceId(required)lang(required, e.g.fr_FR) [1]
2) messageUnitaire (required)
media(required, e.g.SMS)textMsg(required)to(required, destination number) [1]
Optional (commonly used)
from(optional): set sender (e.g., OADC for SMS, if enabled). [2]
Source documentation pages: [1], [2]
🏁 Script executed:
# Find the schema registration for lang, media, and sender fields
sed -n '250,350p' internal/notify/sfr.goRepository: unraid/apprise-go
Length of output: 2647
🏁 Script executed:
# Get the complete schema registration context
sed -n '200,432p' internal/notify/sfr.go | grep -A 8 '"lang"\|"media"\|"sender"'Repository: unraid/apprise-go
Length of output: 891
sender field is incorrectly marked as required—it's optional per the SFR API.
Per the SFR MessagesUnitairesWS/addSingleCall API documentation, from (sender) is optional, but the schema marks "required": true with an empty string default. Update the schema to reflect this:
Fix schema for sender field
"sender": map[string]any{
"default": "",
"map_to": "sender",
"name": "Sender Name",
"private": false,
- "required": true,
+ "required": false,
"type": "string",
},Note: lang and media are correctly marked as required—the SFR API requires both in the authentication and message payloads, and the defaults ("fr_FR" and "SMSUnicode") ensure valid values are always provided.
📝 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.
| "lang": map[string]any{ | |
| "default": "fr_FR", | |
| "map_to": "lang", | |
| "name": "Language", | |
| "private": false, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| "media": map[string]any{ | |
| "default": "SMSUnicode", | |
| "map_to": "media", | |
| "name": "Media Type", | |
| "private": false, | |
| "required": true, | |
| "type": "string", | |
| "values": []string{"SMS", "SMSLong", "SMSUnicode", "SMSUnicodeLong"}, | |
| }, | |
| "overflow": map[string]any{ | |
| "default": "upstream", | |
| "map_to": "overflow", | |
| "name": "Overflow Mode", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"split", "truncate", "upstream"}, | |
| }, | |
| "rto": map[string]any{ | |
| "default": 4, | |
| "map_to": "rto", | |
| "name": "Socket Read Timeout", | |
| "private": false, | |
| "required": false, | |
| "type": "float", | |
| }, | |
| "sender": map[string]any{ | |
| "default": "", | |
| "map_to": "sender", | |
| "name": "Sender Name", | |
| "private": false, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| "lang": map[string]any{ | |
| "default": "fr_FR", | |
| "map_to": "lang", | |
| "name": "Language", | |
| "private": false, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| "media": map[string]any{ | |
| "default": "SMSUnicode", | |
| "map_to": "media", | |
| "name": "Media Type", | |
| "private": false, | |
| "required": true, | |
| "type": "string", | |
| "values": []string{"SMS", "SMSLong", "SMSUnicode", "SMSUnicodeLong"}, | |
| }, | |
| "overflow": map[string]any{ | |
| "default": "upstream", | |
| "map_to": "overflow", | |
| "name": "Overflow Mode", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"split", "truncate", "upstream"}, | |
| }, | |
| "rto": map[string]any{ | |
| "default": 4, | |
| "map_to": "rto", | |
| "name": "Socket Read Timeout", | |
| "private": false, | |
| "required": false, | |
| "type": "float", | |
| }, | |
| "sender": map[string]any{ | |
| "default": "", | |
| "map_to": "sender", | |
| "name": "Sender Name", | |
| "private": false, | |
| "required": false, | |
| "type": "string", | |
| }, |
🤖 Prompt for AI Agents
In `@internal/notify/sfr.go` around lines 280 - 321, The schema entry for the
"sender" field incorrectly sets "required": true; update the "sender" map (the
sender schema in sfr.go) to mark it optional by setting "required": false (keep
the default "" if desired) so it matches the SFR
MessagesUnitairesWS/addSingleCall API where from/sender is optional; ensure only
the "required" boolean is changed and no other fields (name, map_to, type) are
modified.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@internal/notify/schema_inputs.go`:
- Around line 1342-1355: The condition checking both strings.Contains(lower,
"email") && strings.Contains(lower, "from") in shouldSetEmailToken is
unreachable because the earlier strings.Contains(lower, "from") check already
returns true; fix by either removing the redundant combined check or by
reordering logic so the more specific check (email+from) runs before the generic
"from" check—update the function (shouldSetEmailToken) to use either the
simplified checks (keep the individual "from", "addr", token checks and drop the
combined condition) or move the combined email+from condition above the single
"from" check, ensuring behavior remains correct for lower and lowerToken.
- Around line 286-292: Remove the dead function specDefaultAllowNil: locate the
function declaration named specDefaultAllowNil and delete it entirely from the
file (including its signature and body), then run tests/lint to ensure nothing
else references it; no replacement code is needed since it isn't used.
🧹 Nitpick comments (6)
internal/notify/schema_inputs.go (5)
641-660: Consider usingmath.MaxIntfor readability.The bit manipulation
int(^uint(0) >> 1)calculatesMaxIntbut is less readable than using the standard library constant.♻️ Suggested improvement
+import "math" + func matchSchemaTemplates(templates []string, specs map[string]map[string]any, target *ParsedURL) map[string]any { bestScore := -1 - bestMissing := int(^uint(0) >> 1) + bestMissing := math.MaxInt var bestValues map[string]any
760-788: Duplicated logic inensureEmptyKwargsfor different schema groups.The code blocks for the two
caseclauses are identical. Consider consolidating to reduce duplication.♻️ Proposed refactor
func ensureEmptyKwargs(schema string, specs schemaSpecs, kwargs map[string]map[string]string) { switch strings.ToLower(strings.TrimSpace(schema)) { case "json", "jsons", "xml", "xmls", "form", "forms", "apprise", "apprises", "mailgun", "smtp2go", "sparkpost", "sendgrid", "msg91", "ncloud", "nclouds", "nctalk", "nctalks", "opsgenie", "pagertree", "onesignal", - "synology", "synologys": - for name, spec := range specs.kwargs { - mapTo := specMapTo(spec, name) - if mapTo == "" { - continue - } - if _, ok := kwargs[mapTo]; !ok { - kwargs[mapTo] = map[string]string{} - } - } - case "workflow", "workflows": + "synology", "synologys", + "workflow", "workflows": for name, spec := range specs.kwargs { mapTo := specMapTo(spec, name) if mapTo == "" { continue } if _, ok := kwargs[mapTo]; !ok { kwargs[mapTo] = map[string]string{} } } } }
1291-1295:tokenValueMatchesis a no-op stub.This function always returns
trueand suppresses thespecandvaluearguments. If this is intentional scaffolding for future validation, consider adding a TODO comment. Otherwise, it may mislead readers into thinking actual validation occurs.
86-96: Minor: Redundant map copy in kwargs handling.The
sourcemap is checked for emptiness, then all entries are copied toout. Consider simplifying by assigning directly if source will be used as-is, or clarify if the copy is intentional to avoid mutation.♻️ Simplified approach
if len(source) == 0 { continue } - out := map[string]string{} - for key, value := range source { - out[key] = value - } - if len(out) > 0 { - kwargs[mapTo] = out - } + // Copy to avoid mutating original target fields + out := make(map[string]string, len(source)) + for key, value := range source { + out[key] = value + } + kwargs[mapTo] = outThe second
len(out) > 0check is redundant since we already confirmedlen(source) == 0would have continued.
510-528: Consider handlinguinttypes incoerceInt.The function handles
int,int64, andfloat64, but notuintoruint64. If schema values can be unsigned integers (e.g., from JSON unmarshaling on some platforms), these would fall through to string parsing.♻️ Optional enhancement
func coerceInt(raw any) int { switch value := raw.(type) { case int: return value case int64: return int(value) + case uint: + return int(value) + case uint64: + return int(value) case float64: return int(value)internal/notify/schema_inputs_all_parity_test.go (1)
89-90: Loop variable capture is correct but may be unnecessary in Go 1.22+.The
entry := entrypattern is the classic Go idiom for capturing loop variables in closures. If the project targets Go 1.22+, this is no longer required due to the per-iteration loop variable semantics. However, keeping it ensures backward compatibility.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/notify/mastodon.go`:
- Around line 337-343: The "token" field in the Mastodon notifier schema is
incorrectly marked as non-private; locate the map literal that defines the
"token" entry (the map with keys "map_to", "name", "private", "required", "type"
for the "token" field) and change `"private": false` to `"private": true` so the
access token is treated as sensitive and not exposed in logs or UIs.
In `@internal/notify/schema_inputs.go`:
- Around line 1596-1611: Remove the unused dead helper shouldSetBaseURLForMapTo
function from the file: delete the entire function declaration for
shouldSetBaseURLForMapTo (which takes schemaSpecs and mapTo and iterates
specs.args and specs.tokens) and any related unused helper references (e.g.,
specMapTo) if they were only used by this function; run tests/build to ensure
nothing else depends on shouldSetBaseURLForMapTo and clean up any imports or
comments made redundant by its removal.
- Around line 1377-1381: tokenValueMatches currently always returns true,
bypassing token validation used during URL credential and path-segment matching;
either implement validation or mark it as a deliberate placeholder. Update
tokenValueMatches(spec map[string]any, value any) to inspect spec for expected
keys (e.g., "type", "pattern", "enum", "minLength"/"maxLength") and perform type
checks, regex pattern matching, enum membership and length/number bounds against
value, returning true only when all constraints pass; if you intend it as a
placeholder instead, replace the body with a clear TODO comment explaining
expected validation behavior and why it's deferred (and consider removing the
calls at the call sites if validation is unnecessary).
🧹 Nitpick comments (5)
internal/notify/mastodon.go (1)
159-172: Consider extracting schema definition to a separate file or variable.The
init()function contains a large ~200-line schema definition inline. For maintainability, consider extracting this to a separate file (e.g.,mastodon_schema.go) or at least a package-level variable. This would make both the schema and the core logic easier to review and modify independently.internal/notify/schema_inputs.go (4)
86-95: Simplify redundant map copy logic.The copy from
sourcetooutand the subsequent length check are redundant. Sincesourceis already confirmed non-empty (line 86),outwill always be non-empty after the copy.♻️ Suggested simplification
if len(source) == 0 { continue } - out := map[string]string{} - for key, value := range source { - out[key] = value - } - if len(out) > 0 { - kwargs[mapTo] = out - } + kwargs[mapTo] = maps.Clone(source)If
maps.Cloneisn't available (requires Go 1.21+), you can keep the manual copy but remove the redundantlen(out) > 0check since it's guaranteed to be non-empty at that point.
247-252: Reuse the already-computedbaseURLvalue.
baseURLFromParsed(target)is computed at line 185 and assigned tobaseURL. The same computation is repeated here at line 249. Reuse the existing variable.♻️ Suggested fix
case "revolt": if _, ok := values["link"]; !ok { - if baseURL := baseURLFromParsed(target); baseURL != "" { + if baseURL != "" { values["link"] = schemaValueAny(baseURL) } }
418-429: Consider documenting the purpose of these hardcoded mapTo sets.These sets define special handling for specific field mappings. A brief comment explaining why these fields need special treatment would improve maintainability.
734-752: Consider usingmath.MaxIntfor clarity.The bit manipulation
int(^uint(0) >> 1)works correctly butmath.MaxInt(available since Go 1.17) is more readable and self-documenting.♻️ Suggested change
Add to imports:
import "math"Then:
func matchSchemaTemplates(templates []string, specs map[string]map[string]any, target *ParsedURL) map[string]any { bestScore := -1 - bestMissing := int(^uint(0) >> 1) + bestMissing := math.MaxInt var bestValues map[string]any
| "token": map[string]any{ | ||
| "map_to": "token", | ||
| "name": "Access Token", | ||
| "private": false, | ||
| "required": true, | ||
| "type": "string", | ||
| }, |
There was a problem hiding this comment.
Access token should be marked as private.
The token field is marked with "private": false, but access tokens are sensitive credentials that should not be logged or displayed in UIs. Consider setting "private": true to prevent accidental exposure.
🔒 Proposed fix
"token": map[string]any{
"map_to": "token",
"name": "Access Token",
- "private": false,
+ "private": true,
"required": true,
"type": "string",
},📝 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.
| "token": map[string]any{ | |
| "map_to": "token", | |
| "name": "Access Token", | |
| "private": false, | |
| "required": true, | |
| "type": "string", | |
| }, | |
| "token": map[string]any{ | |
| "map_to": "token", | |
| "name": "Access Token", | |
| "private": true, | |
| "required": true, | |
| "type": "string", | |
| }, |
🤖 Prompt for AI Agents
In `@internal/notify/mastodon.go` around lines 337 - 343, The "token" field in the
Mastodon notifier schema is incorrectly marked as non-private; locate the map
literal that defines the "token" entry (the map with keys "map_to", "name",
"private", "required", "type" for the "token" field) and change `"private":
false` to `"private": true` so the access token is treated as sensitive and not
exposed in logs or UIs.
| func tokenValueMatches(spec map[string]any, value any) bool { | ||
| _ = spec | ||
| _ = value | ||
| return true | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'tokenValueMatches' --type=go -C3Repository: unraid/apprise-go
Length of output: 2591
Add TODO comment or implement token validation logic.
The function is actively called at 5 locations (lines 1018, 1041, 1074, 1092, 1146) to validate tokens during URL credential and path segment matching. However, it unconditionally returns true, bypassing all validation. If this is placeholder code for future validation, add a TODO comment. If validation should be performed, implement the check against the spec parameter; if not needed, simplify the call sites to remove these unnecessary checks.
🤖 Prompt for AI Agents
In `@internal/notify/schema_inputs.go` around lines 1377 - 1381, tokenValueMatches
currently always returns true, bypassing token validation used during URL
credential and path-segment matching; either implement validation or mark it as
a deliberate placeholder. Update tokenValueMatches(spec map[string]any, value
any) to inspect spec for expected keys (e.g., "type", "pattern", "enum",
"minLength"/"maxLength") and perform type checks, regex pattern matching, enum
membership and length/number bounds against value, returning true only when all
constraints pass; if you intend it as a placeholder instead, replace the body
with a clear TODO comment explaining expected validation behavior and why it's
deferred (and consider removing the calls at the call sites if validation is
unnecessary).
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/notify/fcm.go (1)
146-157: Critical: Double-nested "notification" key in payload structure.The payload has
"notification": { "notification": { ... } }which is incorrect for FCM's legacy HTTP API. The expected structure is:{ "notification": { "title": "...", "body": "..." }, "to": "..." }This will cause FCM to ignore the notification content.
Proposed fix
payload := map[string]any{ - "notification": map[string]any{ - "notification": map[string]string{ - "title": title, - "body": body, - }, + "notification": map[string]string{ + "title": title, + "body": body, }, }Note: You'll also need to update the type assertion on line 156 accordingly:
if color, ok := f.resolveColor(notifyType); ok { - payload["notification"].(map[string]any)["notification"].(map[string]string)["color"] = color + payload["notification"].(map[string]string)["color"] = color }And similarly for the image assignment on line 166.
🤖 Fix all issues with AI agents
In `@internal/notify/dingtalk.go`:
- Around line 26-32: The runtime validation for the DingTalk secret uses
dingtalkSecretRegex but enforces lowercase only; update the regex to be
case-insensitive so it matches the schema (e.g. compile regex with the
case-insensitive flag or prefix with (?i)), and ensure all runtime checks that
use dingtalkSecretRegex (including the occurrences around the secret variable
and the other spots noted) rely on that updated regex so valid mixed-case
secrets pass validation.
- Around line 86-90: The code is double-escaping the DingTalk signature: when
d.secret != "" the call to d.signature() returns an already-escaped value which
is then escaped again by url.Values.Encode(); update the implementation of
signature() to return the raw Base64 HMAC string (remove any
url.QueryEscape/QueryEscape-like step) and keep the usage in the caller (the
q.Set("timestamp", timestamp) / q.Set("sign", signature) and subsequent
Encode()) so that Encode() performs the single, correct escaping; ensure no
other callers expect a pre-escaped value from signature().
In `@internal/notify/emby.go`:
- Around line 57-59: BuildRequest currently returns an error when e.user is
empty but Send silently returns nil; make behavior consistent by having Send
surface the same error (preferably by calling BuildRequest or performing the
same validation) and returning an error when e.user is empty instead of nil.
Update the Send method (and any related early-return paths around e.user checks)
to return the same fmt.Errorf("missing user") or the BuildRequest error, so
callers always get a clear error for the misconfiguration; ensure you reference
BuildRequest and Send and the e.user check when implementing the fix.
In `@internal/notify/lametric.go`:
- Around line 207-209: The cloud-mode code never sets l.apiKey so the
X-Access-Token header is never added; update the cloud initialization (where
appToken is assigned) to also populate l.apiKey (or else set
headers["X-Access-Token"] = appToken when building headers) so that the header
is present for cloud requests; reference l.apiKey, appToken and
headers["X-Access-Token"] in your change to ensure authentication works in cloud
mode.
In `@internal/notify/mailgun.go`:
- Around line 180-183: The Send method currently returns nil when m.disabled is
true; change it to return the same error as BuildRequest (fmt.Errorf("missing
apikey")) so callers get a clear failure instead of silently skipping delivery;
update MailgunTarget.Send to check m.disabled and return fmt.Errorf("missing
apikey") (and add fmt to imports if needed) to mirror BuildRequest's behavior.
In `@internal/notify/mastodon.go`:
- Around line 79-103: The BuildRequest method on MastodonTarget is only
prepending configured m.targets when the message contains mentions because the
current logic only builds targetSet/filtering inside the mentions conditional;
move or duplicate the target-prepend logic so configured m.targets are prepended
whenever they are not already present in the message. Specifically, in
MastodonTarget.BuildRequest (using mergeTitleBody, extractMastodonMentions,
mentions, targetSet, filtered, status, and m.targets) compute the set of
existing mentions from the message, dedupe m.targets against that set and
prepend any remaining targets to status even when mentions is empty, preserving
the current de-duplication (seen) behavior.
In `@internal/notify/matrix.go`:
- Around line 3-8: matrixSend currently uses http.DefaultClient.Do which has no
timeout; replace that with a dedicated http.Client that sets timeouts (e.g.,
Timeout: 10*time.Second) and use client.Do instead of http.DefaultClient.Do.
Update imports to include time, and create the client either inside matrixSend
or as a package-level var (e.g., httpClient := &http.Client{Timeout: 10 *
time.Second}) so all Matrix HTTP calls use this timed client.
- Around line 13-21: The transaction ID is being reused in token-only auth
because m.transactionIDString is set to matrixFixedTransactionID and
m.transactionID is never incremented due to the condition (m.accessToken !=
m.password); update the Matrix v3 send logic (where m.transactionIDString and
m.transactionID are handled) to stop assigning the fixed
matrixFixedTransactionID for token-only mode and always increment
m.transactionID on each v3 send, then regenerate m.transactionIDString from
m.transactionID for the request; ensure the increment/unconditional update
occurs regardless of whether accessToken equals password so each request gets a
unique txnId.
In `@internal/notify/msteams.go`:
- Around line 196-231: The buildTemplatePayload path handling currently joins a
relative template path to moduleRoot with filepath.Join allowing escapes via
“..”; update MSTeamsTarget.buildTemplatePayload to normalize and validate the
final path: use filepath.Clean on the incoming path, join with moduleRoot, then
compute a safe relative via filepath.Rel (or compare cleaned prefixes) to ensure
the resolved path remains inside moduleRoot before calling os.ReadFile; if the
resolved path is outside moduleRoot, return an error. Reference symbols:
buildTemplatePayload, path, moduleRoot, findModuleRoot, filepath.Join,
filepath.Clean, filepath.Rel, and os.ReadFile.
In `@internal/notify/notifiarr.go`:
- Around line 110-113: NotifiarrTarget.Send currently returns nil when
n.channels is empty, which conflicts with BuildRequest (lines ~87-89) that
returns an error for the same condition; update Send (method
NotifiarrTarget.Send) to return the same error when len(n.channels) == 0 so
callers get consistent behavior (use the same error message/type used by
BuildRequest or a shared sentinel error) rather than silently succeeding.
In `@internal/notify/ntfy.go`:
- Line 4: Remove the hardcoded "None" fallback for empty Basic Auth passwords
and instead validate that when authType == "basic" the n.password is non-empty;
if it is empty return an error (or fail early) instructing the caller to use
Bearer token auth instead. Update the auth header construction code that
currently references n.password and authType (the Basic Auth branch) to perform
this check before encoding (do not substitute "None"), and ensure the Bearer
token branch (token handling) remains unchanged. Make the error clear and
surfaced from the function that constructs/sends the ntfy request so callers
cannot proceed with an empty Basic Auth password.
In `@internal/notify/opsgenie.go`:
- Around line 130-134: The Send method in OpsgenieTarget currently returns nil
for non-"new" actions, causing silent no-ops and inconsistency with
BuildRequest; update Send (function OpsgenieTarget.Send) to handle non-"new"
resolveAction results by either logging a warning via the existing logger and
returning a sentinel error (e.g., ErrActionSkipped) or by returning the same
error BuildRequest produces for unsupported actions so callers can detect
skipped operations; ensure the change references resolveAction and matches the
error semantics used elsewhere to maintain consistency.
🟡 Minor comments (17)
internal/notify/emby.go-427-433 (1)
427-433: Schema declaresuseras not required, but code enforces it.The schema token for
userhas"required": false(line 431), yet bothBuildRequestandSendenforce that user must be set. This mismatch could mislead consumers of the schema who expect optional user support.Either update the schema to mark
useras required, or adjust the runtime behavior to truly support missing users.Proposed fix if user is indeed required
"user": map[string]any{ "map_to": "user", "name": "Username", "private": false, - "required": false, + "required": true, "type": "string", },internal/notify/notifiarr.go-311-314 (1)
311-314: Self-referential alias:apikeyaliases to itself.The
apikeyarg is defined as"alias_of": "apikey", which references itself. Other aliases like"key"and"from"correctly point to different canonical names ("apikey"and"source"respectively).This should either be a full argument definition (like
cto,emojis, etc.) or removed entirely if the intent is to rely solely on the token definition at lines 417-423.Suggested fix (if full definition is intended)
"apikey": map[string]any{ - "alias_of": "apikey", + "map_to": "apikey", + "name": "API Key", + "private": true, + "required": false, + "type": "string", },internal/notify/fcm.go-357-364 (1)
357-364: Typo: Priority argument has incorrect name.The
priorityargument has"name": "Mode"but should be"name": "Priority".Proposed fix
"priority": map[string]any{ "map_to": "priority", - "name": "Mode", + "name": "Priority", "private": false, "required": false, "type": "choice:string", "values": []string{"min", "low", "normal", "high", "max"}, },internal/notify/fcm.go-128-130 (1)
128-130: Inconsistent handling of oauth2 mode betweenBuildRequestandSend.
BuildRequestreturns an error for oauth2 mode (line 114), butSendsilently succeeds with a no-op. This inconsistency could mislead callers into thinking notifications were sent when they weren't.Consider returning an error here as well for consistency, or at minimum logging a warning so the user knows oauth2 is not yet functional.
Suggested fix for consistency
if f.mode == "oauth2" { - return nil + return fmt.Errorf("oauth2 unsupported") }internal/notify/msteams.go-401-404 (1)
401-404: Fix typo inservice_url.
teams.micrsoft.comlooks misspelled and will be a broken link in schema details.🔧 Proposed fix
- "service_url": "https://teams.micrsoft.com/", + "service_url": "https://teams.microsoft.com/",internal/notify/lametric.go-336-451 (1)
336-451: Several schema arguments are declared but not implemented.The schema exposes 9 arguments (
cto,rto,emojis,format,overflow,sound,store,tz,verify) that are not parsed or used inNewLametricTarget. Whilecycles,priority,icon, andicon_typeare properly implemented, these 9 args remain unimplemented. If they are intentionally exposed for Apprise schema compatibility, add a comment explaining this. Otherwise, users configuring these options will not see any effect.internal/notify/rocket.go-53-61 (1)
53-61: Fragile mode inference heuristic.Using
len(password) > 32to distinguish between token and basic mode is unreliable. API tokens can vary in length (some are shorter), and users may have long passwords. This could cause the wrong authentication mode to be selected.Consider requiring explicit mode specification when both token and basic auth are plausible, or document this behavior clearly so users know to specify
modeexplicitly when needed.internal/notify/rocket.go-154-203 (1)
154-203: Silent success when no targets are configured.If
r.users,r.channels, andr.roomsare all empty,sendAuthenticatedcompletes successfully without sending any messages. This could mask configuration errors where the user forgot to specify recipients.Consider returning an error or warning when no targets are provided in authenticated mode.
🛠️ Suggested fix
func (r *RocketChatTarget) sendAuthenticated(body, title string, notifyType NotifyType) error { + if len(r.users) == 0 && len(r.channels) == 0 && len(r.rooms) == 0 { + return fmt.Errorf("no targets specified for authenticated mode") + } + if r.mode == rocketModeBasic {internal/notify/rocket.go-382-469 (1)
382-469: Schema exposes unimplemented options.The schema advertises several arguments (
cto,rto,emojis,format,overflow,store,tz,verify) that are never read from the query parameters and have no corresponding implementation in theRocketChatTargetstruct. Users can specify these options, but they will be silently ignored.Either remove these from the schema or implement support for them. If they're reserved for future use, add a code comment to clarify the intent.
internal/notify/pagertree.go-161-323 (1)
161-323: Remove unused schema arguments or implement them.The schema defines 8 arguments (
cto,rto,emojis,format,overflow,store,tz,verify) that are not read or used anywhere in the implementation. Onlyaction,tags,thirdparty, andurgencyare actually processed. If these are intentionally copied from the Python Apprise schema for future compatibility, add a comment explaining that. Otherwise, remove them from the schema to avoid misleading users about available functionality.internal/notify/notifico.go-129-259 (1)
129-259: Schema args not consumed by NotificoTarget implementation.Lines 134–217 declare args like
cto,rto,verify,format,emojis,overflow,store, andtzvia schemamap_tometadata, butNewNotificoTarget(lines 28–44) only parsescolorandprefixfrom the URL query. The undeclared args will be silently ignored. Consider either wiring them into the target (if intentional) or removing them from the schema to avoid misleading users about available options.internal/notify/guilded.go-201-205 (1)
201-205: Rename the “Discord Flags” label for Guilded clarity.This is user-facing schema text; consider “Guilded Flags” or “Message Flags” to avoid confusion.
💡 Suggested tweak
- "name": "Discord Flags", + "name": "Guilded Flags",internal/notify/mailgun.go-400-407 (1)
400-407: Rename “Verify SSL” to reflect what the flag actually does.This flag controls
o:skip-verification, not TLS. A clearer label will reduce user confusion.✏️ Suggested wording update
- "name": "Verify SSL", + "name": "Recipient Verification",internal/notify/notica.go-77-81 (1)
77-81: Preserve explicit empty passwords for Basic auth.
pass == ""treats an intentionally empty password the same as “missing”, which can alter auth semantics now thatHasPasswordexists.🐛 Proposed fix
type NoticaTarget struct { mode string scheme string host string port int path string token string headers map[string]string user string pass string + hasPass bool } ... return &NoticaTarget{ mode: mode, scheme: scheme, host: host, port: target.Port, path: path, token: token, headers: cloneMap(target.QueryAdd), user: target.User, pass: target.Password, + hasPass: target.HasPassword, }, nil ... if n.user != "" { pass := n.pass - if pass == "" { + if !n.hasPass { pass = "None" } headers["Authorization"] = basicAuthHeader(n.user, pass) }internal/notify/reddit.go-84-85 (1)
84-85: Schema documents configurable parameters that are now ignored.The schema entry (lines 256-263, 329-336) documents
adandrepliesas configurable arguments, but these values are hardcoded here. Users expecting to configure these via query parameters will be surprised when their settings are ignored.Either remove these args from the schema or restore the query parameter parsing:
♻️ Option: Restore query parameter parsing
- advert: false, - sendreply: true, + advert: parseBoolValue(target.Query["ad"], false), + sendreply: parseBoolValue(target.Query["replies"], true),internal/notify/matrix.go-485-491 (1)
485-491:matrixMarkdownBody()andmatrixMarkdownTitleBody()send unescaped content as HTML.Both functions escape the title but leave the body unescaped. Since the format is set to
"org.matrix.custom.html", any HTML or unescaped markdown in the body will execute as HTML in the Matrix client, allowing injection attacks.Either implement markdown-to-HTML conversion or escape the body content like the title:
Minimal safe fallback (escape + line breaks)
func matrixMarkdown(body string) string { - return body + if body == "" { + return "" + } + escaped := matrixEscapeHTML(body, false) + escaped = strings.ReplaceAll(escaped, "\r\n", "\n") + return strings.ReplaceAll(escaped, "\n", "<br/>") }internal/notify/mastodon.go-141-176 (1)
141-176: Guard against mentions inside email addresses/words.
Without a leading-delimiter check, substrings likeuser@example.comcan be parsed as@example, causing unintended mentions to be prefixed.💡 Suggested fix (validate leading delimiter)
for _, index := range indices { start, end := index[0], index[1] if start < 0 || end <= start || end > len(message) { continue } + if start > 0 { + r, _ := utf8.DecodeLastRuneInString(message[:start]) + if !isMentionDelimiter(r) { + continue + } + } if end < len(message) { r, _ := utf8.DecodeRuneInString(message[end:]) if !isMentionDelimiter(r) { continue } }
🧹 Nitpick comments (13)
internal/notify/fcm.go (1)
31-45: Clarifyprojectassignment for oauth2 mode.Setting
project := apiKeyworks for legacy mode, but in oauth2 mode the project ID is typically distinct from credentials. Since oauth2 mode requires akeyfile, the project ID would normally come from that keyfile or be specified separately.Consider extracting project from a dedicated query parameter for oauth2 mode, or document that for oauth2 the project must be passed via the URL path/host.
internal/notify/pushsafer.go (2)
276-295: Prefer exact match before prefix matching to avoid ambiguous inputs.
Prefix matching can map short/typo inputs to unintended sounds. Consider exact lookup first, then optional prefix fallback.♻️ Suggested refinement
func parsePushSaferSound(raw string) *int { raw = strings.TrimSpace(raw) if raw == "" { return nil } if value, err := strconv.Atoi(raw); err == nil { return &value } normalized := strings.ToLower(raw) normalized = strings.ReplaceAll(normalized, "-", "") normalized = strings.ReplaceAll(normalized, "_", "") normalized = strings.ReplaceAll(normalized, " ", "") + if value, ok := pushSaferSoundMap[normalized]; ok { + return &value + } for _, key := range pushSaferSoundOrder { if strings.HasPrefix(key, normalized) { if value, ok := pushSaferSoundMap[key]; ok { return &value } } } return nil }
298-360: Avoid duplicating the sound list in schema values.
The sound list is already defined inpushSaferSoundOrder; reusing it here prevents drift.♻️ Suggested refactor
func init() { + soundValues := append([]string(nil), pushSaferSoundOrder...) RegisterSchemaEntryOrdered(63, SchemaEntry{ "attachment_support": true, "category": "native", "details": map[string]any{ "args": map[string]any{ @@ "sound": map[string]any{ "map_to": "sound", "name": "Sound", "private": false, "required": false, "type": "choice:string", - "values": []string{"silent", "ahem", "applause", "arrow", "baby", "bell", "bicycle", "bike", "boing", "buzzer", "camera", "carhorn", "horn", "cashregister", "chime", "creakydoor", "cuckooclock", "cuckoo", "disconnect", "dog", "doorbell", "fanfare", "gunshot", "honk", "jawharp", "morse", "electric", "radiotuner", "sirens", "militarytrumpets", "military", "trumpets", "ufo", "whahwhah", "whah", "goodye", "hello", "no", "okay", "ok", "ooohhhweee", "warn", "warning", "welcome", "yeah", "yes", "beep", "beep1", "weee", "wee", "cutinout", "flickglass", "laser", "windchime", "echo", "zipper", "hihat", "beep2", "beep3", "beep4", "alarmarmed", "armed", "alarmdisarmed", "disarmed", "backupready", "dooropen", "dopen", "doorclosed", "dclosed", "windowopen", "wopen", "windowclosed", "wclosed", "lighton", "lon", "lightoff", "loff", "doorbellrang"}, + "values": soundValues, },internal/notify/home_assistant.go (1)
186-193: Regex format with separate flag element may need documentation.The
regexfield uses[]string{"^[a-z0-9_-]+$", "i"}where"i"appears to be a case-insensitivity flag stored as a separate array element. This is an unusual pattern—consider adding a brief comment explaining this format for future maintainers, or verify this matches the expected schema consumer format.internal/notify/rocket.go (3)
279-283: Simplify redundant return.The error check and return can be simplified.
♻️ Suggested simplification
- if err := SendRequest(spec); err != nil { - return err - } - return nil + return SendRequest(spec)
330-332: Minor: prefernilover empty slice literals.Using
var channels []stringornilis idiomatic and avoids unnecessary allocations when slices remain empty.appendwill allocate on first use.♻️ Suggested change
- channels := []string{} - rooms := []string{} - users := []string{} + var channels, rooms, users []string
370-376: Misleading function naming obscures reusable generic logic.
rocketSenddelegates tomatrixSend, but the function is actually a generic HTTP request handler (protocol-agnostic). ThematrixSendfunction takes a genericRequestSpec, makes an HTTP request, parses the JSON response, and returns the result—with no Matrix-specific logic. While the reuse is safe, the naming suggests Matrix-specific code, which could confuse maintainers.Consider renaming
matrixSendto something generic likesendHTTPRequestto clarify that this is protocol-agnostic logic, or add a comment explaining why it's safe to reuse across services.internal/notify/office365.go (2)
124-136: Consider caching failed sender resolution attempts to avoid repeated API calls.If
resolveFromEmail()fails (due to API errors, permissions, or invalid responses), everySend()call will retry the lookup. Since the method doesn't return errors and silently fails,o.fromEmailremains unset, triggering another resolution attempt on the next call. Consider memoizing a failed resolution attempt with a flag or lightweight retry logic to avoid redundant Microsoft Graph API calls, especially in scenarios where permissions prevent user profile lookups.
249-260: Remove unnecessary body and Content-Type header from GET request.GET requests should not include a request body or Content-Type header. Per HTTP semantics (RFC 9110), these can cause issues with proxies and strict servers. Since this request has no payload, remove
Body: "null"and theContent-Typeheader.🔧 Suggested adjustment
spec := RequestSpec{ Method: "GET", URL: fmt.Sprintf("%s/v1.0/users/%s", office365GraphURL, o.source), Headers: map[string]string{ "User-Agent": "Apprise", "Accept": "*/*", - "Content-Type": "application/json", "Authorization": fmt.Sprintf("Bearer %s", o.token), }, - Body: "null", + Body: "", }internal/notify/pagertree.go (2)
133-135: Payload extras can overwrite core fields.The
payloadextras are merged after the core payload fields (id,event_type,title,description,meta,tags,urgency) are set. This allows users to overwrite critical fields, which could lead to unexpected behavior or malformed requests.If this flexibility is intentional, consider documenting it. Otherwise, consider either:
- Applying payload extras before setting core fields, or
- Skipping reserved keys when merging
♻️ Option: Skip reserved keys
+ reservedKeys := map[string]bool{ + "id": true, "event_type": true, "title": true, + "description": true, "meta": true, "tags": true, "urgency": true, + } for key, value := range p.payload { + if reservedKeys[key] { + continue + } payload[key] = value }
144-151: Custom headers can override critical defaults.Custom headers from
QueryAddcan overwriteContent-Type: application/json. If a user overrides this (e.g., totext/plain), the server may fail to parse the JSON body. This flexibility may be intentional, but consider documenting this behavior or protectingContent-Type.internal/notify/ntfy.go (1)
223-223: Consider adding support for configurable base paths if subpath hosting is a requirement.Currently,
notifyURL()hardcodesPath: "/". While this is appropriate for ntfy's protocol (topics are sent in the JSON body), it prevents deploying apprise under a reverse-proxied subpath (e.g.,/ntfy/) if the server expects the client to know about the base path. Standard reverse proxy setups with URL rewriting should work fine, but if ntfy or users need explicit base path support, consider adding a parameter toNtfyTargetto preserve or override the path.internal/cli/storage.go (1)
213-221: Silently ignoring deletion errors may hide permission or filesystem issues.All
os.Removecalls discard errors. If deletions fail due to permissions or other issues, users won't know their prune/clear operation was incomplete.Consider collecting failures and reporting a summary, or at least logging when
doDeleteis true but removal fails.
| if d.secret != "" { | ||
| timestamp, signature := d.signature() | ||
| q.Set("timestamp", timestamp) | ||
| q.Set("sign", signature) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "dingtalk.go" -type fRepository: unraid/apprise-go
Length of output: 90
🏁 Script executed:
cat -n internal/notify/dingtalk.go | head -150Repository: unraid/apprise-go
Length of output: 4614
🌐 Web query:
Go net/url package url.Values.Encode QueryEscape escaping values
💡 Result:
In Go’s net/url, url.Values.Encode() builds an application/x-www-form-urlencoded query string by escaping each key and each value with url.QueryEscape, then joining as k=v pairs with & (keys are sorted). You can see this in the Values.Encode() example output (cat sounds → cat+sounds, / → %2F, $ → %24). [1]
That means the escaping rules you’ll observe for values are the same as QueryEscape:
- space becomes
+(form-style encoding, not%20) - literal
+becomes%2B - many reserved/special bytes become
%XX(e.g.&→%26,=→%3D) [2]
On decode, QueryUnescape reverses QueryEscape, and (unlike PathUnescape) it treats + as a space. [2]
If you need RFC 3986-style spaces (%20) instead of form-style +, Values.Encode() won’t produce that directly—you typically post-process the output (replace + with %20) or use different escaping depending on the URL component (e.g., PathEscape for path segments). [2]
Sources:
[1] https://pkg.go.dev/net/url (Values.Encode example/output)
[2] https://pkg.go.dev/net/url (QueryEscape/QueryUnescape + PathUnescape note)
Avoid double-encoding the DingTalk signature.
url.Values.Encode() automatically applies QueryEscape to all values. The signature() function is pre-escaping the Base64 signature, causing it to be escaped twice: once in signature() and again in Encode(). This breaks authentication since DingTalk receives an invalidly formatted signature. Return the raw Base64 signature and let Encode() handle all escaping.
Fix
- signature := base64.StdEncoding.EncodeToString(mac.Sum(nil))
- return timestamp, url.QueryEscape(signature)
+ signature := base64.StdEncoding.EncodeToString(mac.Sum(nil))
+ return timestamp, signature🤖 Prompt for AI Agents
In `@internal/notify/dingtalk.go` around lines 86 - 90, The code is
double-escaping the DingTalk signature: when d.secret != "" the call to
d.signature() returns an already-escaped value which is then escaped again by
url.Values.Encode(); update the implementation of signature() to return the raw
Base64 HMAC string (remove any url.QueryEscape/QueryEscape-like step) and keep
the usage in the caller (the q.Set("timestamp", timestamp) / q.Set("sign",
signature) and subsequent Encode()) so that Encode() performs the single,
correct escaping; ensure no other callers expect a pre-escaped value from
signature().
| if l.apiKey != "" { | ||
| headers["X-Access-Token"] = l.apiKey | ||
| } |
There was a problem hiding this comment.
Bug: Cloud mode X-Access-Token header never set.
In cloud mode, l.apiKey is never populated (see lines 147-157 where only appToken is set). This condition will always be false, causing cloud mode requests to fail authentication.
🐛 Proposed fix
- if l.apiKey != "" {
- headers["X-Access-Token"] = l.apiKey
+ if l.appToken != "" {
+ headers["X-Access-Token"] = l.appToken
}📝 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 l.apiKey != "" { | |
| headers["X-Access-Token"] = l.apiKey | |
| } | |
| if l.appToken != "" { | |
| headers["X-Access-Token"] = l.appToken | |
| } |
🤖 Prompt for AI Agents
In `@internal/notify/lametric.go` around lines 207 - 209, The cloud-mode code
never sets l.apiKey so the X-Access-Token header is never added; update the
cloud initialization (where appToken is assigned) to also populate l.apiKey (or
else set headers["X-Access-Token"] = appToken when building headers) so that the
header is present for cloud requests; reference l.apiKey, appToken and
headers["X-Access-Token"] in your change to ensure authentication works in cloud
mode.
| func (m *MailgunTarget) Send(body, title string, notifyType NotifyType) error { | ||
| if m.disabled { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find the file and understand the structure
find . -name "*.go" -path "*/notify/*" | head -20Repository: unraid/apprise-go
Length of output: 677
🏁 Script executed:
#!/bin/bash
# Run the verification script provided in the comment
rg -n -C3 'disabled bool' --type=go
rg -n -C3 'if .*disabled' --type=go
rg -n -C3 'Send\(' --type=go internal/notifyRepository: unraid/apprise-go
Length of output: 47082
🏁 Script executed:
#!/bin/bash
# Let's examine the mailgun.go file to see BuildRequest error handling
cat -n internal/notify/mailgun.go | head -250Repository: unraid/apprise-go
Length of output: 7535
Mirror BuildRequest's error handling in Send for consistency.
Send returns nil when disabled is true, but BuildRequest returns fmt.Errorf("missing apikey") for the same condition. Callers using Send directly will silently skip delivery without knowing a required API key is missing. Change Send to return the same error as BuildRequest:
Suggested fix
func (m *MailgunTarget) Send(body, title string, notifyType NotifyType) error {
if m.disabled {
- return nil
+ return fmt.Errorf("missing apikey")
}📝 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.
| func (m *MailgunTarget) Send(body, title string, notifyType NotifyType) error { | |
| if m.disabled { | |
| return nil | |
| } | |
| func (m *MailgunTarget) Send(body, title string, notifyType NotifyType) error { | |
| if m.disabled { | |
| return fmt.Errorf("missing apikey") | |
| } |
🤖 Prompt for AI Agents
In `@internal/notify/mailgun.go` around lines 180 - 183, The Send method currently
returns nil when m.disabled is true; change it to return the same error as
BuildRequest (fmt.Errorf("missing apikey")) so callers get a clear failure
instead of silently skipping delivery; update MailgunTarget.Send to check
m.disabled and return fmt.Errorf("missing apikey") (and add fmt to imports if
needed) to mirror BuildRequest's behavior.
| const ( | ||
| matrixWebhookPath = "/api/v1/matrix/hook" | ||
| matrixV2APIPath = "/_matrix/client/r0" | ||
| matrixV3APIPath = "/_matrix/client/v3" | ||
| matrixV2MediaPath = "/_matrix/media/r0" | ||
| matrixV3MediaPath = "/_matrix/media/v3" | ||
| matrixT2BotWebhookURL = "https://webhooks.t2bot.io/api/v1/matrix/hook/" | ||
| matrixDefaultUserAgent = "Apprise" | ||
| matrixFixedTransactionID = "00000000-0000-4000-8000-000000000000" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -name "matrix.go" -path "*/notify/*" 2>/dev/null | head -5Repository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
# Locate the file first
find . -name "matrix.go" -type f 2>/dev/null | grep -E "(notify|internal)" | head -5Repository: unraid/apprise-go
Length of output: 88
🏁 Script executed:
# Get repository structure to locate the file
ls -la internal/notify/ 2>/dev/null | head -20 || echo "Trying different path..."
git ls-files | grep -i matrixRepository: unraid/apprise-go
Length of output: 1382
🏁 Script executed:
# Read the specific lines mentioned in the review
cat -n internal/notify/matrix.go | sed -n '13,21p'Repository: unraid/apprise-go
Length of output: 556
🏁 Script executed:
# Read lines around the transaction ID assignments
cat -n internal/notify/matrix.go | sed -n '195,210p'Repository: unraid/apprise-go
Length of output: 559
🏁 Script executed:
# Read the increment logic mentioned
cat -n internal/notify/matrix.go | sed -n '490,515p'Repository: unraid/apprise-go
Length of output: 1089
🌐 Web query:
Matrix v3 client API transaction ID de-duplication idempotency
💡 Result:
Matrix Client-Server API (v3) uses a client-generated transaction ID (txnId) on certain PUT endpoints (notably event sending) to provide idempotency / de-duplication:
- How de-duplication works: A homeserver should treat a request as a retransmission (duplicate) when both the
txnIdand the HTTP path match a previous request, and then return the exact same HTTP status + response body as the original (e.g., the sameevent_idfor/rooms/{roomId}/send/.../{txnId}). (spec.matrix.org) - Client requirements: Clients should generate a unique
txnIdper request (recommended: UUIDv4, or timestamp + monotonic counter) and change it for the next request once the request completes. (spec.matrix.org) - Scope rules (what counts as “the same”):
- Scope is per device and per endpoint (so the same
txnIdcan be reused across different endpoints without being considered a duplicate). (spec.matrix.org) - If you refresh an access token and reuse a
txnIdfor the same endpoint/device, it’s still treated as a duplicate. (spec.matrix.org) - If you log out and back in, that typically creates a new device, so the same
txnIdwould be considered distinct unless you log back in reusing the samedevice_id. (spec.matrix.org)
- Scope is per device and per endpoint (so the same
- Event sending endpoint:
PUT /_matrix/client/v3/rooms/{roomId}/send/{eventType}/{txnId}explicitly statestxnId“will be used by the server to ensure idempotency”. (spec.matrix.org)
References (numbered):
[1] Matrix Spec (latest) — Transaction identifiers (spec.matrix.org)
[2] Matrix Spec — PUT /_matrix/client/v3/rooms/{roomId}/send/{eventType}/{txnId} (txnId for idempotency) (spec.matrix.org)
Citations:
- 1: https://spec.matrix.org/latest/client-server-api/
- 2: https://spec.matrix.org/latest/client-server-api/
- 3: https://spec.matrix.org/latest/client-server-api/
- 4: https://spec.matrix.org/latest/client-server-api/
- 5: https://spec.matrix.org/latest/client-server-api/
- 6: https://spec.matrix.org/latest/client-server-api/index.html?utm_source=openai
- 7: https://spec.matrix.org/latest/client-server-api/
- 8: https://spec.matrix.org/latest/client-server-api/index.html?utm_source=openai
🏁 Script executed:
# Check the authentication flow and how token-only auth is used
cat -n internal/notify/matrix.go | sed -n '180,210p'Repository: unraid/apprise-go
Length of output: 1114
🏁 Script executed:
# Check the login and register functions to understand token-only flow
cat -n internal/notify/matrix.go | sed -n '220,250p'Repository: unraid/apprise-go
Length of output: 1091
Fix transaction ID reuse in token-only auth mode for Matrix v3.
When only a password is provided (token-only auth), the code sets m.transactionIDString to a fixed constant and never increments m.transactionID. This causes all subsequent messages to reuse the same transaction ID. Per the Matrix v3 spec, the server treats requests with the same txnId on the same endpoint as retransmissions and returns the same event_id—effectively dropping message 2 onwards.
The increment condition at line 495 guards on m.accessToken != m.password, which is false in token-only mode, preventing the counter from ever advancing. Always increment m.transactionID for v3 sends and drop the fixed constant.
🐛 Proposed fix
- matrixFixedTransactionID = "00000000-0000-4000-8000-000000000000"- m.transactionIDString = matrixFixedTransactionID- if ok && m.version == matrixVersionV3 && m.accessToken != "" && m.accessToken != m.password && m.transactionIDString == "" {
+ if ok && m.version == matrixVersionV3 && m.transactionIDString == "" {
m.transactionID++
}🤖 Prompt for AI Agents
In `@internal/notify/matrix.go` around lines 13 - 21, The transaction ID is being
reused in token-only auth because m.transactionIDString is set to
matrixFixedTransactionID and m.transactionID is never incremented due to the
condition (m.accessToken != m.password); update the Matrix v3 send logic (where
m.transactionIDString and m.transactionID are handled) to stop assigning the
fixed matrixFixedTransactionID for token-only mode and always increment
m.transactionID on each v3 send, then regenerate m.transactionIDString from
m.transactionID for the request; ensure the increment/unconditional update
occurs regardless of whether accessToken equals password so each request gets a
unique txnId.
| func (m *MSTeamsTarget) buildTemplatePayload(body, title string, notifyType NotifyType) (map[string]any, error) { | ||
| path := strings.TrimSpace(m.templatePath) | ||
| if strings.HasPrefix(path, "file://") { | ||
| path = strings.TrimPrefix(path, "file://") | ||
| } | ||
| if path != "" && !filepath.IsAbs(path) { | ||
| if moduleRoot, ok := findModuleRoot(); ok { | ||
| path = filepath.Join(moduleRoot, path) | ||
| } | ||
| } | ||
| data, err := os.ReadFile(path) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
|
|
||
| tokens := map[string]string{} | ||
| for key, value := range m.templateTokens { | ||
| tokens[key] = value | ||
| } | ||
| tokens["app_body"] = body | ||
| tokens["app_title"] = title | ||
| tokens["app_type"] = string(notifyType) | ||
| tokens["app_id"] = "Apprise" | ||
| tokens["app_desc"] = "Apprise Notifications" | ||
| tokens["app_color"] = appriseColor(notifyType) | ||
| tokens["app_image_url"] = appriseImageURL(notifyType, "72x72") | ||
| tokens["app_url"] = appriseAppURL | ||
| tokens["app_mode"] = "json" | ||
|
|
||
| rendered := applyTemplateTokens(string(data), tokens) | ||
|
|
||
| var payload map[string]any | ||
| if err := json.Unmarshal([]byte(rendered), &payload); err != nil { | ||
| return nil, err | ||
| } | ||
| return payload, nil |
There was a problem hiding this comment.
Guard relative template paths from escaping module root.
When template is relative, filepath.Join(moduleRoot, path) allows .. to escape the module root (e.g., ../secrets.json). If the intent is to confine template files to the module root, normalize and enforce the final path stays within the root.
🔒 Suggested fix (normalize + enforce within module root)
if path != "" && !filepath.IsAbs(path) {
if moduleRoot, ok := findModuleRoot(); ok {
- path = filepath.Join(moduleRoot, path)
+ cleaned := filepath.Clean(path)
+ joined := filepath.Join(moduleRoot, cleaned)
+ rel, err := filepath.Rel(moduleRoot, joined)
+ if err != nil || strings.HasPrefix(rel, "..") {
+ return nil, fmt.Errorf("template path escapes module root")
+ }
+ path = joined
}
}📝 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.
| func (m *MSTeamsTarget) buildTemplatePayload(body, title string, notifyType NotifyType) (map[string]any, error) { | |
| path := strings.TrimSpace(m.templatePath) | |
| if strings.HasPrefix(path, "file://") { | |
| path = strings.TrimPrefix(path, "file://") | |
| } | |
| if path != "" && !filepath.IsAbs(path) { | |
| if moduleRoot, ok := findModuleRoot(); ok { | |
| path = filepath.Join(moduleRoot, path) | |
| } | |
| } | |
| data, err := os.ReadFile(path) | |
| if err != nil { | |
| return nil, err | |
| } | |
| tokens := map[string]string{} | |
| for key, value := range m.templateTokens { | |
| tokens[key] = value | |
| } | |
| tokens["app_body"] = body | |
| tokens["app_title"] = title | |
| tokens["app_type"] = string(notifyType) | |
| tokens["app_id"] = "Apprise" | |
| tokens["app_desc"] = "Apprise Notifications" | |
| tokens["app_color"] = appriseColor(notifyType) | |
| tokens["app_image_url"] = appriseImageURL(notifyType, "72x72") | |
| tokens["app_url"] = appriseAppURL | |
| tokens["app_mode"] = "json" | |
| rendered := applyTemplateTokens(string(data), tokens) | |
| var payload map[string]any | |
| if err := json.Unmarshal([]byte(rendered), &payload); err != nil { | |
| return nil, err | |
| } | |
| return payload, nil | |
| func (m *MSTeamsTarget) buildTemplatePayload(body, title string, notifyType NotifyType) (map[string]any, error) { | |
| path := strings.TrimSpace(m.templatePath) | |
| if strings.HasPrefix(path, "file://") { | |
| path = strings.TrimPrefix(path, "file://") | |
| } | |
| if path != "" && !filepath.IsAbs(path) { | |
| if moduleRoot, ok := findModuleRoot(); ok { | |
| cleaned := filepath.Clean(path) | |
| joined := filepath.Join(moduleRoot, cleaned) | |
| rel, err := filepath.Rel(moduleRoot, joined) | |
| if err != nil || strings.HasPrefix(rel, "..") { | |
| return nil, fmt.Errorf("template path escapes module root") | |
| } | |
| path = joined | |
| } | |
| } | |
| data, err := os.ReadFile(path) | |
| if err != nil { | |
| return nil, err | |
| } | |
| tokens := map[string]string{} | |
| for key, value := range m.templateTokens { | |
| tokens[key] = value | |
| } | |
| tokens["app_body"] = body | |
| tokens["app_title"] = title | |
| tokens["app_type"] = string(notifyType) | |
| tokens["app_id"] = "Apprise" | |
| tokens["app_desc"] = "Apprise Notifications" | |
| tokens["app_color"] = appriseColor(notifyType) | |
| tokens["app_image_url"] = appriseImageURL(notifyType, "72x72") | |
| tokens["app_url"] = appriseAppURL | |
| tokens["app_mode"] = "json" | |
| rendered := applyTemplateTokens(string(data), tokens) | |
| var payload map[string]any | |
| if err := json.Unmarshal([]byte(rendered), &payload); err != nil { | |
| return nil, err | |
| } | |
| return payload, nil | |
| } |
🤖 Prompt for AI Agents
In `@internal/notify/msteams.go` around lines 196 - 231, The buildTemplatePayload
path handling currently joins a relative template path to moduleRoot with
filepath.Join allowing escapes via “..”; update
MSTeamsTarget.buildTemplatePayload to normalize and validate the final path: use
filepath.Clean on the incoming path, join with moduleRoot, then compute a safe
relative via filepath.Rel (or compare cleaned prefixes) to ensure the resolved
path remains inside moduleRoot before calling os.ReadFile; if the resolved path
is outside moduleRoot, return an error. Reference symbols: buildTemplatePayload,
path, moduleRoot, findModuleRoot, filepath.Join, filepath.Clean, filepath.Rel,
and os.ReadFile.
| package notify | ||
|
|
||
| import ( | ||
| "encoding/base64" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n internal/notify/ntfy.go | sed -n '140,155p'Repository: unraid/apprise-go
Length of output: 616
🌐 Web query:
ntfy basic authentication empty password allowed
💡 Result:
In ntfy, an empty Basic-Auth password (i.e., Authorization: Basic base64("user:") or curl -u user:) is not a special/allowed mode—it’s just “password = empty string”, and it will only authenticate if that user’s actual password is empty (which ntfy does not document/recommend as a normal setup).
If what you want is “no username/password, just one secret value”, use an access token instead: ntfy explicitly supports Basic Auth with an empty username, where the Basic-Auth password is treated as the access token, e.g. curl -u :tk_... [1].
Also note: some clients/configs mention “empty password” only as a client-side YAML representation (set the password to ""), i.e., “send an empty password”, not “ntfy will accept empty passwords in general” [2].
Sources:
[1] ntfy docs – Publishing / Authentication (empty username → password treated as token) https://docs.ntfy.sh/publish/
[2] Example ntfy client.yml comment (“For an empty password, use empty double-quotes ("")”) https://nicholaswilde.io/homelab/apps/ntfy/
Remove the "None" fallback and add validation for non-empty password in Basic Auth.
The hardcoded "None" replacement (line 148) is a workaround for an unsupported scenario: ntfy does not accept empty passwords in Basic Auth. If a user intends to use only a token, they should use the Bearer token auth method (already supported in the code at line 152–153) instead.
Either validate that n.password is non-empty when authType == "basic", or document this requirement. Simply removing the fallback without validation will result in failed authentication with empty passwords.
🤖 Prompt for AI Agents
In `@internal/notify/ntfy.go` at line 4, Remove the hardcoded "None" fallback for
empty Basic Auth passwords and instead validate that when authType == "basic"
the n.password is non-empty; if it is empty return an error (or fail early)
instructing the caller to use Bearer token auth instead. Update the auth header
construction code that currently references n.password and authType (the Basic
Auth branch) to perform this check before encoding (do not substitute "None"),
and ensure the Bearer token branch (token handling) remains unchanged. Make the
error clear and surfaced from the function that constructs/sends the ntfy
request so callers cannot proceed with an empty Basic Auth password.
| func (o *OpsgenieTarget) Send(body, title string, notifyType NotifyType) error { | ||
| action := o.resolveAction(notifyType) | ||
| if action != "new" { | ||
| return fmt.Errorf("unsupported action: %s", action) | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Silent no-op may mislead callers expecting action execution.
For non-"new" actions (close, delete, acknowledge, note), Send now silently returns success without performing any operation. This has two concerns:
- Users may believe their action was executed when nothing happened
- Inconsistency:
BuildRequestreturns an error for the same actions (line 164), butSendreturns success
Consider logging a warning or returning a sentinel that indicates the action was skipped, rather than silently succeeding.
🤖 Prompt for AI Agents
In `@internal/notify/opsgenie.go` around lines 130 - 134, The Send method in
OpsgenieTarget currently returns nil for non-"new" actions, causing silent
no-ops and inconsistency with BuildRequest; update Send (function
OpsgenieTarget.Send) to handle non-"new" resolveAction results by either logging
a warning via the existing logger and returning a sentinel error (e.g.,
ErrActionSkipped) or by returning the same error BuildRequest produces for
unsupported actions so callers can detect skipped operations; ensure the change
references resolveAction and matches the error semantics used elsewhere to
maintain consistency.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@internal/notify/notifiarr.go`:
- Around line 311-314: The "args" map contains a self-referential alias entry
`"apikey": map[string]any{"alias_of":"apikey"}` — remove this entry (or replace
it with the correct alias name if you intended a different alias) so it no
longer aliases to itself; locate the "args" map in notifiarr.go and remove the
`"apikey"` alias entry (note that the canonical "apikey" token is already
defined later in the file) to match the project's aliasing pattern.
In `@internal/notify/pushed.go`:
- Around line 180-252: The schema lists args (cto, rto, emojis, format,
overflow, store, tz, verify) but NewPushedTarget and the payload-building code
in internal/notify/pushed.go don't read or apply them; either remove these keys
from the schema or parse and wire them through: update NewPushedTarget (or the
factory that constructs PushedTarget) to extract these keys from the config/map
(use existing pattern for other args), store them on the PushedTarget struct,
and modify the payload construction and send logic to apply them (socket
connect/read timeouts cto/rto, SSL verify flag, emojis flag, format and overflow
handling, persistent store flag, and tz). Locate references to NewPushedTarget
and the payload construction block in pushed.go to add parsing, struct fields,
and usage, or alternatively delete the unused arg entries from the schema block
to keep it accurate.
- Around line 12-13: The pushedChannelRe currently allows an optional '#' (var
pushedChannelRe = regexp.MustCompile(`^#?([A-Za-z0-9]+)$`)) which contradicts
the Pushed.co API and schema; update pushedChannelRe to require the '#' prefix
(use `^#([A-Za-z0-9]+)$`) so channels must start with '#' like pushedUserRe
requires '@', ensuring bare alphanumeric strings are not treated as channels.
- Around line 47-50: BuildRequest on PushedTarget currently always returns only
an app-level request by calling buildPayload and buildRequest, ignoring
PushedTarget.channels and PushedTarget.users; align it with Send by detecting
configured channels and users and producing additional RequestSpec entries with
target_type "channel" for each channel and "pushed_id" (or "user") for each
user, or alternatively document the limitation if you prefer not to change
behavior. Update PushedTarget.BuildRequest to iterate PushedTarget.channels and
PushedTarget.users, call p.buildPayload(...) per target (or reuse payload where
appropriate) and call p.buildRequest(...) for each, returning the combined
RequestSpec(s) or change the method signature to return multiple RequestSpec and
adjust callers accordingly; reference PushedTarget.Send,
PushedTarget.buildPayload and PushedTarget.buildRequest when making the change.
- Around line 254-257: The template
"{schema}://{app_key}/{app_secret}@{targets}" in the templates slice causes
app_secret to include targets because '@' isn't a path separator; update the
templates entry to use a slash separator
"{schema}://{app_key}/{app_secret}/{targets}" (or alternatively add explicit
handling for '@' in the path parsing logic used by splitPath()), so change the
templates array in internal/notify/pushed.go and/or adjust the parsing in
splitPath() to correctly separate app_secret and targets.
🧹 Nitpick comments (6)
internal/cli/config.go (2)
63-66: Scanner error handling returns partial results silently.When
scanner.Err()returns an error (e.g., due to a line exceeding buffer size), the function returns partial results without logging or returning the error. This could make debugging difficult if config files are unexpectedly truncated.♻️ Consider logging or returning the error
if err := scanner.Err(); err != nil { + // Optionally log: log.Printf("warning: config file scan error: %v", err) return urls } return urls
248-257: Tilde expansion doesn't support~usersyntax.The function handles
~and~/pathbut not~username/path(e.g.,~bob/.apprise). This is likely intentional given the use case, but worth noting if users expect POSIX-style expansion.internal/cli/storage.go (1)
300-323: Consider using binary unit suffixes (KiB, MiB, etc.).The function uses 1024-based divisions but labels units as KB, MB, GB. Technically, these SI prefixes denote 1000-based units. The binary equivalents are KiB, MiB, GiB. This is a minor convention detail that most users won't notice.
internal/cli/cli.go (1)
196-211: TODO comments note unimplemented options.These options (
inputFormat,disableAsync,attachments, etc.) are parsed but explicitly marked as not yet wired into the runtime. This is acceptable for incremental development but should be tracked.Would you like me to open issues to track implementation of these TODO items?
internal/notify/pushed.go (2)
35-37: Edge case: all-whitespace targets silently succeed.If a user provides targets that are all empty or whitespace (e.g.,
to=,,,),parsePushedTargetsskips them without settinghasInvalid, so the conditionlen(targets) > 0 && ... && hasInvalidis false and no error is returned. The result is a target with no channels/users, which falls back to app-level notification. This may be acceptable, but consider whether you want to treat "non-empty input that yields zero valid targets" as an error.
67-67: Consider documenting whynotifyTypeis unused.The blank identifier assignment
_ = notifyTypesuppresses the unused parameter warning, but a brief comment explaining why Pushed doesn't use notification types would help future maintainers understand this is intentional rather than a TODO.
| "args": map[string]any{ | ||
| "apikey": map[string]any{ | ||
| "alias_of": "apikey", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# View the context around the suspicious lines
cat -n internal/notify/notifiarr.go | sed -n '305,320p'Repository: unraid/apprise-go
Length of output: 579
🏁 Script executed:
# View the canonical token definition area
cat -n internal/notify/notifiarr.go | sed -n '415,430p'Repository: unraid/apprise-go
Length of output: 618
🏁 Script executed:
# Search for all alias_of patterns in notification files
rg -n '"alias_of"' internal/notify/ --type goRepository: unraid/apprise-go
Length of output: 10587
🏁 Script executed:
# Check other notification implementations for schema patterns
fd -e go . internal/notify/ | head -5 | xargs -I {} bash -c 'echo "=== {} ===" && rg -n "RegisterSchemaEntry" {} | head -2'Repository: unraid/apprise-go
Length of output: 335
🏁 Script executed:
cat -n internal/notify/notifiarr.go | sed -n '390,400p'Repository: unraid/apprise-go
Length of output: 391
🏁 Script executed:
# Check if apikey appears in both args and tokens sections
cat -n internal/notify/notifiarr.go | sed -n '311,315p' && echo "---" && cat -n internal/notify/notifiarr.go | sed -n '417,423p'Repository: unraid/apprise-go
Length of output: 429
🏁 Script executed:
# Check what happens with targets field
cat -n internal/notify/notifiarr.go | sed -n '392,396p'Repository: unraid/apprise-go
Length of output: 200
Remove or fix the self-referential alias for apikey at lines 312-314.
The entry "apikey": map[string]any{"alias_of": "apikey"} is self-referential and inconsistent with the codebase pattern. In 200+ alias definitions across 80+ notification files, every single alias points to a different canonical field (e.g., "key" → "apikey", "from" → "source"). Since apikey is already a canonical token at lines 417-423, this entry in the args section should either be removed entirely or its alias_of value corrected to reference a different field.
🤖 Prompt for AI Agents
In `@internal/notify/notifiarr.go` around lines 311 - 314, The "args" map contains
a self-referential alias entry `"apikey": map[string]any{"alias_of":"apikey"}` —
remove this entry (or replace it with the correct alias name if you intended a
different alias) so it no longer aliases to itself; locate the "args" map in
notifiarr.go and remove the `"apikey"` alias entry (note that the canonical
"apikey" token is already defined later in the file) to match the project's
aliasing pattern.
| var pushedChannelRe = regexp.MustCompile(`^#?([A-Za-z0-9]+)$`) | ||
| var pushedUserRe = regexp.MustCompile(`^@([A-Za-z0-9]+)$`) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the file to see the full context
cat -n internal/notify/pushed.go | head -100Repository: unraid/apprise-go
Length of output: 3117
🏁 Script executed:
# Search for how parsePushedTargets uses these regexes
rg -A 20 "func parsePushedTargets" internal/notify/pushed.goRepository: unraid/apprise-go
Length of output: 620
🌐 Web query:
Pushed.co API targets format channel identifier requirements documentation
💡 Result:
In Pushed.co targeting, the “target” is expressed as either:
- App (all subscribers of the app): no explicit target segment is added (just app key/secret). (confluxbot.com)
- Specific user(s) by Pushed ID: each user target must be prefixed with
@(otherwise it’s ignored). (confluxbot.com) - Specific channel(s) by channel alias: each channel target must be prefixed with
#(otherwise it’s ignored), and it must be the channel alias from your channel settings (not the channel name). (confluxbot.com)
Common documented target formats (often used by integrations/wrappers):
pushed://{app_key}/{app_secret}pushed://{app_key}/{app_secret}/@{user_pushed_id}(repeat@...for multiple users)pushed://{app_key}/{app_secret}/#{channel_alias}(repeat#...for multiple channels)- You can combine users + channels:
pushed://{app_key}/{app_secret}/@{user_pushed_id}/#{channel_alias}/(confluxbot.com)
Citations:
- 1: https://www.confluxbot.com/docs/platforms/magic_account/pushed/?utm_source=openai
- 2: https://www.confluxbot.com/docs/platforms/magic_account/pushed/?utm_source=openai
- 3: https://www.confluxbot.com/docs/platforms/magic_account/pushed/?utm_source=openai
- 4: https://www.confluxbot.com/docs/platforms/magic_account/pushed/?utm_source=openai
🏁 Script executed:
# Check if there's any code that adds the # prefix back later
rg -A 10 "Send\|buildRequest\|buildPayload" internal/notify/pushed.go | grep -A 10 "target_type\|channel\|user"Repository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
# Check if there's schema validation or any other logic
cat -n internal/notify/pushed.go | tail -50Repository: unraid/apprise-go
Length of output: 1633
Fix channel regex to require mandatory # prefix per API specification.
The pushedChannelRe pattern ^#?([A-Za-z0-9]+)$ makes the # prefix optional. This contradicts both the Pushed.co API specification (which requires # for channels, otherwise ignored) and the schema definition (which declares "prefix": "#"). Bare alphanumeric strings like foo will be classified as channels instead of invalid, leading to silent failures when the API ignores them. Change to ^#([A-Za-z0-9]+)$ to match the API and schema requirements, consistent with the user regex requiring @.
🤖 Prompt for AI Agents
In `@internal/notify/pushed.go` around lines 12 - 13, The pushedChannelRe
currently allows an optional '#' (var pushedChannelRe =
regexp.MustCompile(`^#?([A-Za-z0-9]+)$`)) which contradicts the Pushed.co API
and schema; update pushedChannelRe to require the '#' prefix (use
`^#([A-Za-z0-9]+)$`) so channels must start with '#' like pushedUserRe requires
'@', ensuring bare alphanumeric strings are not treated as channels.
| func (p *PushedTarget) BuildRequest(body, title string, notifyType NotifyType) (RequestSpec, error) { | ||
| payload := p.buildPayload(body, title, notifyType) | ||
| return p.buildRequest(payload) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find all usages of BuildRequest and surrounding context
rg -n "BuildRequest" --type=go -C3Repository: unraid/apprise-go
Length of output: 50373
🏁 Script executed:
# Examine the PushedTarget type and its methods
cd internal/notify && fd -e go -x cat -n {} | head -200Repository: unraid/apprise-go
Length of output: 6639
🏁 Script executed:
# Look for interface definitions that might include BuildRequest
rg -n "interface.*{" --type=go -A10 internal/notify/Repository: unraid/apprise-go
Length of output: 148
🏁 Script executed:
# Read the full pushed.go file to understand buildPayload and buildRequest helpers
cat -n internal/notify/pushed.goRepository: unraid/apprise-go
Length of output: 9865
🏁 Script executed:
# Search for interface definitions that include BuildRequest
rg -n "interface" --type=go -A20 | grep -A20 "BuildRequest" | head -40Repository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
# Look for NotifyTarget or similar interface that might define BuildRequest
rg -n "type.*interface" --type=go -A15 internal/notify/Repository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
# Search for interface definition that might define BuildRequest
rg -n "^type.*Target.*interface" --type=goRepository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
# Look for places where BuildRequest is called outside of Send methods
rg -n "\.BuildRequest\(" --type=go -B2 -A2 | grep -v "func.*Send" | head -50Repository: unraid/apprise-go
Length of output: 2167
🏁 Script executed:
# Check if there's a NotifyTarget interface or similar
rg -n "type.*interface.*{" --type=go -A20 | head -100Repository: unraid/apprise-go
Length of output: 160
🏁 Script executed:
# Search for interface definitions more simply
rg -n "interface\s*{" --type=go -B2 -A10Repository: unraid/apprise-go
Length of output: 150
🏁 Script executed:
# Look for where NotifyTarget might be defined or used
rg -n "NotifyTarget\|type.*Target.*struct" --type=go | head -40Repository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
# Check if BuildRequest appears outside of Send implementations or tests
rg -n "BuildRequest" --type=go | grep -v "Send\|func.*BuildRequest\|apprise_api\|ntfy\|notifico\|pushplus\|ryver\|sendpulse\|techulus\|xml\|zulip\|wecombot\|pushed" | head -20Repository: unraid/apprise-go
Length of output: 1761
BuildRequest always produces app-level request only—channels and users are ignored.
This method hardcodes target_type: "app" and never reads the channels or users fields configured on the target. The Send method handles these properly by creating separate requests with target_type: "channel" and target_type: "pushed_id", but BuildRequest does not. External callers using BuildRequest directly (e.g., for inspection or testing) will get only the app-level request. Document this limitation or align BuildRequest with Send behavior.
🤖 Prompt for AI Agents
In `@internal/notify/pushed.go` around lines 47 - 50, BuildRequest on PushedTarget
currently always returns only an app-level request by calling buildPayload and
buildRequest, ignoring PushedTarget.channels and PushedTarget.users; align it
with Send by detecting configured channels and users and producing additional
RequestSpec entries with target_type "channel" for each channel and "pushed_id"
(or "user") for each user, or alternatively document the limitation if you
prefer not to change behavior. Update PushedTarget.BuildRequest to iterate
PushedTarget.channels and PushedTarget.users, call p.buildPayload(...) per
target (or reuse payload where appropriate) and call p.buildRequest(...) for
each, returning the combined RequestSpec(s) or change the method signature to
return multiple RequestSpec and adjust callers accordingly; reference
PushedTarget.Send, PushedTarget.buildPayload and PushedTarget.buildRequest when
making the change.
| "details": map[string]any{ | ||
| "args": map[string]any{ | ||
| "cto": map[string]any{ | ||
| "default": 4.0, | ||
| "map_to": "cto", | ||
| "name": "Socket Connect Timeout", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "float", | ||
| }, | ||
| "emojis": map[string]any{ | ||
| "default": false, | ||
| "map_to": "emojis", | ||
| "name": "Interpret Emojis", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| "format": map[string]any{ | ||
| "default": "text", | ||
| "map_to": "format", | ||
| "name": "Notify Format", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"html", "markdown", "text"}, | ||
| }, | ||
| "overflow": map[string]any{ | ||
| "default": "upstream", | ||
| "map_to": "overflow", | ||
| "name": "Overflow Mode", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"split", "truncate", "upstream"}, | ||
| }, | ||
| "rto": map[string]any{ | ||
| "default": 4.0, | ||
| "map_to": "rto", | ||
| "name": "Socket Read Timeout", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "float", | ||
| }, | ||
| "store": map[string]any{ | ||
| "default": true, | ||
| "map_to": "store", | ||
| "name": "Persistent Storage", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| "to": map[string]any{ | ||
| "alias_of": "targets", | ||
| "delim": []string{",", " "}, | ||
| }, | ||
| "tz": map[string]any{ | ||
| "default": nil, | ||
| "map_to": "tz", | ||
| "name": "Timezone", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "string", | ||
| }, | ||
| "verify": map[string]any{ | ||
| "default": true, | ||
| "map_to": "verify", | ||
| "name": "Verify SSL", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "bool", | ||
| }, | ||
| }, |
There was a problem hiding this comment.
Schema advertises args that aren't implemented.
The schema declares configuration options like cto, rto, emojis, format, overflow, store, tz, and verify, but NewPushedTarget and the payload construction don't parse or use any of these. Users configuring these options will have no effect. Either implement support for these options or remove them from the schema to avoid misleading users.
#!/bin/bash
# Verify if any of these args are parsed elsewhere
rg -n "emojis|overflow|store|verify|cto|rto" internal/notify/pushed.go🤖 Prompt for AI Agents
In `@internal/notify/pushed.go` around lines 180 - 252, The schema lists args
(cto, rto, emojis, format, overflow, store, tz, verify) but NewPushedTarget and
the payload-building code in internal/notify/pushed.go don't read or apply them;
either remove these keys from the schema or parse and wire them through: update
NewPushedTarget (or the factory that constructs PushedTarget) to extract these
keys from the config/map (use existing pattern for other args), store them on
the PushedTarget struct, and modify the payload construction and send logic to
apply them (socket connect/read timeouts cto/rto, SSL verify flag, emojis flag,
format and overflow handling, persistent store flag, and tz). Locate references
to NewPushedTarget and the payload construction block in pushed.go to add
parsing, struct fields, and usage, or alternatively delete the unused arg
entries from the schema block to keep it accurate.
| "templates": []string{ | ||
| "{schema}://{app_key}/{app_secret}", | ||
| "{schema}://{app_key}/{app_secret}@{targets}", | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, let's find ParsedURL/ParseURL usage and implementation
rg -n "ParsedURL|ParseURL" internal/ -A 5 -B 2Repository: unraid/apprise-go
Length of output: 50373
🏁 Script executed:
cat -n internal/notify/pushed.go | head -300 | tail -100Repository: unraid/apprise-go
Length of output: 3385
🏁 Script executed:
sed -n '952,1020p' internal/notify/schema_inputs.goRepository: unraid/apprise-go
Length of output: 2147
🏁 Script executed:
sed -n '952,1100p' internal/notify/schema_inputs.goRepository: unraid/apprise-go
Length of output: 3862
🏁 Script executed:
sed -n '29,150p' internal/notify/url.goRepository: unraid/apprise-go
Length of output: 2819
🏁 Script executed:
sed -n '1,100p' internal/notify/pushed.goRepository: unraid/apprise-go
Length of output: 2417
🏁 Script executed:
rg -n "pushed" internal/notify -i --type go -l | head -10Repository: unraid/apprise-go
Length of output: 119
🏁 Script executed:
sed -n '1,100p' internal/notify/registry_push.goRepository: unraid/apprise-go
Length of output: 1797
🏁 Script executed:
sed -n '150,250p' internal/notify/url.goRepository: unraid/apprise-go
Length of output: 2307
🏁 Script executed:
cd /tmp && cat > test_url.go << 'EOF'
package main
import (
"fmt"
"strings"
)
func main() {
// Simulate template expansion
template1 := "pushed://{app_key}/{app_secret}@{targets}"
// Replace placeholders
url1 := strings.ReplaceAll(template1, "{app_key}", "mykey")
url1 = strings.ReplaceAll(url1, "{app_secret}", "mysecret")
url1 = strings.ReplaceAll(url1, "{targets}", "target1")
fmt.Printf("Template: %s\n", template1)
fmt.Printf("Filled: %s\n", url1)
fmt.Println()
// Show what authority extraction does
parts := strings.SplitN(url1, "://", 2)
if len(parts) == 2 {
rest := parts[1]
var authority string
for i, ch := range rest {
if ch == '/' || ch == '?' || ch == '#' {
authority = rest[:i]
break
}
}
if authority == "" {
authority = rest
}
fmt.Printf("Authority part: %s\n", authority)
fmt.Printf("@ count in authority: %d\n", strings.Count(authority, "@"))
if idx := strings.Index(rest, "/"); idx != -1 {
path := rest[idx:]
fmt.Printf("Path part: %s\n", path)
fmt.Printf("@ in path: %v\n", strings.Contains(path, "@"))
}
}
}
EOF
go run test_url.goRepository: unraid/apprise-go
Length of output: 362
🏁 Script executed:
cat > /tmp/test_parse.go << 'EOF'
package main
import (
"fmt"
"net/url"
"strings"
)
func main() {
// The expanded URL from template
testURL := "pushed://mykey/mysecret@target1"
fmt.Printf("Test URL: %s\n\n", testURL)
// Standard Go URL parser
u, err := url.Parse(testURL)
if err != nil {
fmt.Printf("Parse error: %v\n", err)
} else {
fmt.Println("Standard url.Parse result:")
fmt.Printf(" Scheme: %s\n", u.Scheme)
fmt.Printf(" User: %v\n", u.User)
fmt.Printf(" Hostname: %s\n", u.Hostname())
fmt.Printf(" Host: %s\n", u.Host)
fmt.Printf(" Path: %s\n", u.Path)
fmt.Printf(" RawPath: %s\n", u.RawPath)
}
fmt.Println("\n--- What if @ was in authority? ---")
testURL2 := "pushed://mykey:mysecret@target1"
u2, err := url.Parse(testURL2)
if err != nil {
fmt.Printf("Parse error: %v\n", err)
} else {
fmt.Println("Standard url.Parse result:")
fmt.Printf(" Scheme: %s\n", u2.Scheme)
if u2.User != nil {
fmt.Printf(" User: %s\n", u2.User.Username())
if pw, ok := u2.User.Password(); ok {
fmt.Printf(" Password: %s\n", pw)
}
}
fmt.Printf(" Hostname: %s\n", u2.Hostname())
fmt.Printf(" Host: %s\n", u2.Host)
fmt.Printf(" Path: %s\n", u2.Path)
}
}
EOF
go run /tmp/test_parse.goRepository: unraid/apprise-go
Length of output: 141
🏁 Script executed:
cat > /tmp/test_parse.go << 'EOF'
package main
import (
"fmt"
"net/url"
)
func main() {
// The expanded URL from template
testURL := "pushed://mykey/mysecret@target1"
fmt.Printf("Test URL: %s\n\n", testURL)
// Standard Go URL parser
u, err := url.Parse(testURL)
if err != nil {
fmt.Printf("Parse error: %v\n", err)
} else {
fmt.Println("Standard url.Parse result:")
fmt.Printf(" Scheme: %s\n", u.Scheme)
fmt.Printf(" User: %v\n", u.User)
fmt.Printf(" Hostname: %s\n", u.Hostname())
fmt.Printf(" Host: %s\n", u.Host)
fmt.Printf(" Path: %s\n", u.Path)
fmt.Printf(" RawPath: %s\n", u.RawPath)
}
fmt.Println("\n--- What if @ was in authority? ---")
testURL2 := "pushed://mykey:mysecret@target1"
u2, err := url.Parse(testURL2)
if err != nil {
fmt.Printf("Parse error: %v\n", err)
} else {
fmt.Println("Standard url.Parse result:")
fmt.Printf(" Scheme: %s\n", u2.Scheme)
if u2.User != nil {
fmt.Printf(" User: %s\n", u2.User.Username())
if pw, ok := u2.User.Password(); ok {
fmt.Printf(" Password: %s\n", pw)
}
}
fmt.Printf(" Hostname: %s\n", u2.Hostname())
fmt.Printf(" Host: %s\n", u2.Host)
fmt.Printf(" Path: %s\n", u2.Path)
}
}
EOF
go run /tmp/test_parse.goRepository: unraid/apprise-go
Length of output: 387
🏁 Script executed:
rg -n "func splitPath" internal/notify/ -A 10Repository: unraid/apprise-go
Length of output: 1991
🏁 Script executed:
cat > /tmp/test_pushed.go << 'EOF'
package main
import (
"fmt"
"strings"
)
func splitPath(pathValue string) []string {
if pathValue == "" {
return nil
}
trimmed := strings.TrimLeft(pathValue, "/")
if trimmed == "" {
return nil
}
parts := strings.Split(trimmed, "/")
segments := make([]string, 0, len(parts))
for _, part := range parts {
if part == "" {
continue
}
segments = append(segments, part)
}
return segments
}
func main() {
// Simulate what happens with the template expansion
testURL := "pushed://mykey/mysecret@target1"
fmt.Printf("Test URL: %s\n\n", testURL)
// Simulate what ParsedURL would contain after parsing
host := "mykey"
path := "/mysecret@target1"
fmt.Printf("After URL parsing:\n")
fmt.Printf(" Host: %s\n", host)
fmt.Printf(" Path: %s\n", path)
fmt.Println()
// Now simulate NewPushedTarget logic
appKey := host
segments := splitPath(path)
fmt.Printf("In NewPushedTarget:\n")
fmt.Printf(" appKey: %s\n", appKey)
fmt.Printf(" segments: %v\n", segments)
if len(segments) > 0 {
appSecret := segments[0]
fmt.Printf(" appSecret: %s\n", appSecret)
fmt.Printf(" ERROR: appSecret includes `@target1`!\n")
}
}
EOF
go run /tmp/test_pushed.goRepository: unraid/apprise-go
Length of output: 298
🏁 Script executed:
rg -n "pushed" internal/ -A 5 -B 5 | grep -A 5 -B 5 "@{targets}"Repository: unraid/apprise-go
Length of output: 43
🏁 Script executed:
find . -name "*test*.go" -type f | xargs grep -l "pushed" 2>/dev/nullRepository: unraid/apprise-go
Length of output: 104
Template @ separator causes incorrect parsing of app_secret.
The template {schema}://{app_key}/{app_secret}@{targets} has a design flaw. When expanded with actual values (e.g., pushed://mykey/mysecret@target1), the URL parser places the @{targets} portion in the path rather than as a URL authority delimiter. The subsequent splitPath() call on /mysecret@target1 returns ["mysecret@target1"] as a single segment because @ is not a path separator. This causes app_secret to be parsed as mysecret@target1, contaminating the secret with target values. Use / as the separator instead: {schema}://{app_key}/{app_secret}/{targets}, or implement explicit handling for @ in the path-based parsing logic.
🤖 Prompt for AI Agents
In `@internal/notify/pushed.go` around lines 254 - 257, The template
"{schema}://{app_key}/{app_secret}@{targets}" in the templates slice causes
app_secret to include targets because '@' isn't a path separator; update the
templates entry to use a slash separator
"{schema}://{app_key}/{app_secret}/{targets}" (or alternatively add explicit
handling for '@' in the path parsing logic used by splitPath()), so change the
templates array in internal/notify/pushed.go and/or adjust the parsing in
splitPath() to correctly separate app_secret and targets.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@internal/notify/fcm_oauth.go`:
- Around line 136-176: The fetchFCMAccessToken function uses http.DefaultClient
which has no timeout; replace the call to http.DefaultClient.Do with a dedicated
http.Client that has a bounded Timeout (e.g. client := &http.Client{Timeout: 10
* time.Second}) and call client.Do(req) instead, adding the necessary time
import; alternatively you can create a context with timeout
(context.WithTimeout) and attach it to the request via req =
req.WithContext(ctx) before performing the request, but ensure the request is
cancelled on timeout and resp.Body is still closed.
- Around line 42-55: In buildOAuthSpec (FCMTarget.buildOAuthSpec) remove the
pre-populated "token": nil from the message map so you don't emit a null value;
instead create the base message with only "notification" and then set either
"topic" (strings.TrimPrefix(recipient, "#")) or "token" (recipient) depending on
whether recipient starts with "#" so exactly one of those fields is present and
the marshalled JSON contains no nulls.
In `@internal/notify/fcm.go`:
- Around line 388-395: The schema entry for the "priority" field currently sets
its display label to "Mode"; update that label to "Priority" by changing the
"name" value in the "priority" map literal (the map keyed by "priority"
containing "map_to", "name", "private", "required", "type", "values") so the
user-facing label reads "Priority" instead of "Mode".
| "priority": map[string]any{ | ||
| "map_to": "priority", | ||
| "name": "Mode", | ||
| "private": false, | ||
| "required": false, | ||
| "type": "choice:string", | ||
| "values": []string{"min", "low", "normal", "high", "max"}, | ||
| }, |
There was a problem hiding this comment.
Schema label typo: “priority” is named “Mode”.
This is user-facing and should read “Priority”.
✏️ Proposed fix
- "name": "Mode",
+ "name": "Priority",📝 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.
| "priority": map[string]any{ | |
| "map_to": "priority", | |
| "name": "Mode", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"min", "low", "normal", "high", "max"}, | |
| }, | |
| "priority": map[string]any{ | |
| "map_to": "priority", | |
| "name": "Priority", | |
| "private": false, | |
| "required": false, | |
| "type": "choice:string", | |
| "values": []string{"min", "low", "normal", "high", "max"}, | |
| }, |
🤖 Prompt for AI Agents
In `@internal/notify/fcm.go` around lines 388 - 395, The schema entry for the
"priority" field currently sets its display label to "Mode"; update that label
to "Priority" by changing the "name" value in the "priority" map literal (the
map keyed by "priority" containing "map_to", "name", "private", "required",
"type", "values") so the user-facing label reads "Priority" instead of "Mode".
Remove the nil token field from the FCM OAuth payload. Use a dedicated HTTP client with a 10s timeout for token fetch.
> [!IMPORTANT] > Merging this pull request will create this release ## Features - cli add schema details and storage support (#24) Co-authored-by: knope-bot[bot] <152252888+knope-bot[bot]@users.noreply.github.com>
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.