fix(synology-chat): add SSRF guard to sendFileUrl [AI-assisted]#69603
fix(synology-chat): add SSRF guard to sendFileUrl [AI-assisted]#69603pgondhi987 wants to merge 26 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR adds a Confidence Score: 5/5Safe to merge — the SSRF guard is correct, fail-closed, and consistent with the codebase pattern; only a minor missing test for IPv6 loopback remains. The No files require special attention. Prompt To Fix All With AIThis is a comment left during a code review.
Path: extensions/synology-chat/src/client.test.ts
Line: 192-214
Comment:
**Missing IPv6 loopback test case**
The suite tests `[::]` (unspecified) but not `[::1]` (loopback). `isPrivateOrLoopbackHost` in `gateway/net.ts` does handle bracketed IPv6 correctly via `parseHostForAddressChecks`, so `[::1]` is blocked, but an explicit test would lock in the integration between the guard and the shared helper for this common loopback form.
```suggestion
it("blocks IPv6 unspecified address file URLs before reaching the NAS webhook", async () => {
const result = await settleTimers(
sendFileUrl("https://nas.example.com/incoming", "http://[::]/api/internal"),
);
expect(result).toBe(false);
expect(vi.mocked(https.request)).not.toHaveBeenCalled();
});
it("blocks IPv6 loopback file URLs before reaching the NAS webhook", async () => {
const result = await settleTimers(
sendFileUrl("https://nas.example.com/incoming", "http://[::1]/api/internal"),
);
expect(result).toBe(false);
expect(vi.mocked(https.request)).not.toHaveBeenCalled();
});
it("blocks non-http file URLs before reaching the NAS webhook", async () => {
const result = await settleTimers(
sendFileUrl("https://nas.example.com/incoming", "file:///etc/passwd"),
);
expect(result).toBe(false);
expect(vi.mocked(https.request)).not.toHaveBeenCalled();
});
it("blocks malformed file URLs before reaching the NAS webhook", async () => {
const result = await settleTimers(
sendFileUrl("https://nas.example.com/incoming", "not a url at all"),
);
expect(result).toBe(false);
expect(vi.mocked(https.request)).not.toHaveBeenCalled();
});
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (1): Last reviewed commit: "fix: address review feedback" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 58c3e6a8fa
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 256659e2b4
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 193daea801
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e673af9687
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6bd47b228a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1dc6fb55e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9b8fff306
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0ba29ea320
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29856e5c8b
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a034145d44
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 270f14941d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5f7d6abb4c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46d3a3e2e5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41f34c2c8a
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5fe75814ae
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (routeRegistrationFailed) { | ||
| unregisterWebhookRoute(); | ||
| unregisterMediaRoute(); | ||
| return () => {}; |
There was a problem hiding this comment.
If the hosted-media prefix route fails to register (for example, a conflicting prefix already exists), routeRegistrationFailed causes both unregister callbacks to run, which tears down the already-registered webhook route too. That makes inbound Synology messages stop working even though only hosted-media setup failed; before this change, webhook handling could still continue. This should only disable hosted media (or log/degrade), not remove the primary webhook route.
Useful? React with 👍 / 👎.
1 participant
Summary
sendFileUrlin the Synology Chat extension passed thefileUrlparameter directly to the NAS incoming webhook asfile_urlwith no URL validation, protocol restriction, or private-IP check. The NAS then fetches that URL from its network position, enabling SSRF against the NAS's internal network (cloud metadata endpoints, RFC1918 hosts, loopback services).isSafeWebhookFileUrl()guard inextensions/synology-chat/src/client.tsthat rejects non-http/https protocols, empty hostnames, private/loopback IP ranges (via the sharedisPrivateOrLoopbackHosthelper fromopenclaw/plugin-sdk/ssrf-runtime), and the unspecified addresses0.0.0.0/[::]which are not covered by the shared helper.sendFileUrlreturnsfalseimmediately for any URL that fails the guard.sendMessage,fetchChatUsers,resolveLegacyWebhookNameToChatUserId, and theincomingUrl(NAS webhook) code path are untouched. No behavior change for legitimate public file URLs.Change Type (select all)
Scope (select all touched areas)
Linked Issue/PR
Root Cause (if applicable)
sendFileUrlwas added without applying the SSRF guard pattern used elsewhere in the codebase for media/file URL forwarding.file_urlwebhook payload.Regression Test Plan (if applicable)
extensions/synology-chat/src/client.test.tssendFileUrlreturnsfalseand never callshttps.requestfor loopback IPs,localhost, private-network IPs,0.0.0.0,[::], non-http schemes, and malformed URLs.sendFileUrlboundary fully cover the decision logic.New test cases added:
http://127.0.0.1:8080/...(IPv4 loopback)http://localhost/...(loopback hostname)http://192.168.1.10/...(RFC1918 private)http://0.0.0.0/...(unspecified address)http://[::]/...(IPv6 unspecified, bracketed form)file:///etc/passwd(non-http scheme)not a url at all(malformed URL)User-visible / Behavior Changes
Attempts to send a file URL pointing to a private/loopback/unspecified address or a non-http(s) URL via the Synology Chat extension will silently return
false(no message sent) instead of forwarding the URL to the NAS. Legitimate public file URLs are unaffected.Diagram (if applicable)
Security Impact (required)
new URL()normalizes before the hostname check. IPv6 bracket stripping is explicit to handle WHATWG URL parser behavior.Repro + Verification
Environment
Steps
sendFileUrlwith a private/loopback/unspecified/non-http file URLfalsewithout making any network callExpected
sendFileUrlreturnsfalsehttps.requestis not calledActual (after fix)
Evidence
Seven new unit tests were added covering the full blocked-URL surface. All pass.
Human Verification (required)
isBlockedSpecialIpLiteralcorrectly strips IPv6 brackets beforenet.isIPcheck (runtime-verified via Node.js REPL during review).[::]bracketed IPv6 unspecified (WHATWG URL parser returns"[::]", not"::");0.0.0.0unspecified not covered by shared helper;localhosthostname; malformed URL parse failure path.Review Conversations
Compatibility / Migration
fileUrlinputs that previously forwarded uncheckedRisks and Mitigations
fileUrlthat resolves via internal DNS to a public IP is blocked at the hostname level (no DNS resolution at guard time).