fix(synology-chat): support JSON/alias payloads and ACK webhook with 204#26635
Merged
steipete merged 2 commits intoopenclaw:mainfrom Mar 2, 2026
Merged
Conversation
Comment on lines
+106
to
+108
| const bearerMatch = auth.match(/^Bearer\s+(.+)$/i); | ||
| if (bearerMatch?.[1]) return bearerMatch[1].trim(); | ||
| return auth.trim(); |
Contributor
There was a problem hiding this comment.
Non-Bearer auth header used as raw token
When the Authorization header is present but uses a scheme other than Bearer (e.g. Basic dXNlcjpwYXNz), the entire header value (including the scheme prefix like Basic ) is used as the token. This will silently fail validateToken — which is safe — but it's surprising behavior that could mask configuration mistakes. Consider returning undefined when the header doesn't match the Bearer pattern instead of falling back to the raw value:
Suggested change
| const bearerMatch = auth.match(/^Bearer\s+(.+)$/i); | |
| if (bearerMatch?.[1]) return bearerMatch[1].trim(); | |
| return auth.trim(); | |
| const bearerMatch = auth.match(/^Bearer\s+(.+)$/i); | |
| if (bearerMatch?.[1]) return bearerMatch[1].trim(); | |
| return undefined; |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix With AI
This is a comment left during a code review.
Path: extensions/synology-chat/src/webhook-handler.ts
Line: 106-108
Comment:
**Non-Bearer auth header used as raw token**
When the `Authorization` header is present but uses a scheme other than `Bearer` (e.g. `Basic dXNlcjpwYXNz`), the entire header value (including the scheme prefix like `Basic `) is used as the token. This will silently fail `validateToken` — which is safe — but it's surprising behavior that could mask configuration mistakes. Consider returning `undefined` when the header doesn't match the `Bearer` pattern instead of falling back to the raw value:
```suggestion
const bearerMatch = auth.match(/^Bearer\s+(.+)$/i);
if (bearerMatch?.[1]) return bearerMatch[1].trim();
return undefined;
```
<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>
How can I resolve this? If you propose a fix, please make it concise.d660156 to
aa42241
Compare
Contributor
|
Landed via temp rebase onto main.
Thanks @memphislee09-source! |
This was referenced Mar 2, 2026
Closed
execute008
pushed a commit
to execute008/openclaw
that referenced
this pull request
Mar 2, 2026
dawi369
pushed a commit
to dawi369/davis
that referenced
this pull request
Mar 3, 2026
OWALabuy
pushed a commit
to kcinzgg/openclaw
that referenced
this pull request
Mar 4, 2026
zooqueen
pushed a commit
to hanzoai/bot
that referenced
this pull request
Mar 6, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a Synology Chat webhook interoperability issue that can leave users stuck on
Processing....This patch updates the Synology webhook handler to:
application/x-www-form-urlencodedandapplication/jsonbody.token,query.token, or headersuser_id <- user_id | userId | usertext <- text | message | content204 No Contentinstead of returning{"text":"Processing..."}Why
In real Synology setups, outgoing webhook payload shape/content-type can vary. Strict body-only form parsing caused missing-field errors and UI stalls.
Implementation notes
webhook-handler.tsTests
pnpm vitest run extensions/synology-chat/src/webhook-handler.test.tsCloses #26628
Greptile Summary
This PR improves the Synology Chat webhook handler to accept both
application/jsonandapplication/x-www-form-urlencodedpayloads, support field aliases (userId/user,message/content), resolve tokens from body, query params, or headers, and ACK requests with204 No Contentinstead of a200with{"text":"Processing..."}that could stall the Synology Chat UI.extractTokenFromHeadersfalls back to using the rawAuthorizationheader value (including scheme prefix) as the token when it doesn't matchBearer. This should returnundefinedinstead to avoid masking misconfigurationsConfidence Score: 4/5
extensions/synology-chat/src/webhook-handler.ts— review theextractTokenFromHeadersnon-Bearer fallback on line 108Last reviewed commit: d660156
(2/5) Greptile learns from your feedback when you react with thumbs up/down!