Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,15 @@ nowo_html_to_word:
profiles:
default:
strict_mode: false
images:
# Keep remote HTTP(S) image fetching off by default (SSRF surface).
resolve_remote: false
# When enabling resolve_remote, set a tight remote_host_allowlist (empty deny-all).
# remote_host_allowlist: ['cdn.example.com']

when@prod:
nowo_html_to_word:
profiles:
default:
images:
resolve_remote: false
4 changes: 2 additions & 2 deletions docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ It does **not** expose HTTP routes by itself; embedding apps choose authorizatio
### SSRF via remote images

- **Risk**: When `images.resolve_remote` is true, `file_get_contents()` may fetch attacker-controlled URLs from the server network.
- **Mitigation**: Keep remote resolution **disabled** in sensitive deployments; terminate outbound traffic at the network layer; prefer base64 or controlled CDN URLs at the application layer. HTTP timeouts are limited (10s) in `ImageResolver`.
- **Mitigation**: Keep remote resolution **disabled** (recipe default and `when@prod: resolve_remote: false`). `RemoteImageHostPolicy` **denies** remote hosts when `remote_host_allowlist` is empty — configure an explicit allowlist before enabling remote. Terminate outbound traffic at the network layer; prefer base64 or controlled CDN URLs at the application layer. HTTP timeouts are limited (10s) in `ImageResolver`.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Describe 10 seconds as the default timeout, not a limit.

ImageResolver reads images.remote_timeout and uses 10.0 only when the setting is absent. The code enforces a minimum of 0.1 seconds, but it does not enforce a maximum of 10 seconds. Update this sentence to avoid misleading operators.

Proposed wording
-- HTTP timeouts are limited (10s) in `ImageResolver`.
+- HTTP timeouts default to 10s and can be configured with `images.remote_timeout` in `ImageResolver`.

As per path instructions, user-facing documentation in English under docs/ must not invent features, versions, or commands.

📝 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.

Suggested change
- **Mitigation**: Keep remote resolution **disabled** (recipe default and `when@prod: resolve_remote: false`). `RemoteImageHostPolicy` **denies** remote hosts when `remote_host_allowlist` is empty — configure an explicit allowlist before enabling remote. Terminate outbound traffic at the network layer; prefer base64 or controlled CDN URLs at the application layer. HTTP timeouts are limited (10s) in `ImageResolver`.
- **Mitigation**: Keep remote resolution **disabled** (recipe default and `when@prod: resolve_remote: false`). `RemoteImageHostPolicy` **denies** remote hosts when `remote_host_allowlist` is empty — configure an explicit allowlist before enabling remote. Terminate outbound traffic at the network layer; prefer base64 or controlled CDN URLs at the application layer. HTTP timeouts default to 10s and can be configured with `images.remote_timeout` in `ImageResolver`.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/SECURITY.md` at line 52, Update the ImageResolver timeout statement in
the security documentation to describe 10 seconds as the default used when no
images.remote_timeout setting is provided, not as an enforced limit. Preserve
the surrounding mitigation guidance and avoid implying a maximum timeout.

Source: Path instructions


### Path traversal / local file read via `<img src>`

Expand Down Expand Up @@ -110,4 +110,4 @@ Record confirmation in the release PR or tag notes.
| Grade | Pass (conditional) |
| Risk | Medium |
| Method | Cursor security-review / campaign static pass (`src/`, Flex recipe, demo, SECURITY docs) |
| Open residuals | No Critical/High. **Accepted Medium:** SSRF surface if `images.resolve_remote: true` without a tight `remote_host_allowlist` and network egress controls; empty allowlist is permissive when remote is enabled. Prefer default `resolve_remote: false`. App-owned: HTML size limits, path validation for local `<img src>`, avoid logging full HTML or credentialed remote URLs. |
| Open residuals | No Critical/High. **Accepted Medium:** SSRF surface if `images.resolve_remote: true` without a tight `remote_host_allowlist` and network egress controls. An **empty** `remote_host_allowlist` is **deny-all** (`RemoteImageHostPolicy`); configure an explicit allowlist when enabling remote. Flex recipe keeps `resolve_remote: false` (including `when@prod`). App-owned: HTML size limits, path validation for local `<img src>`, avoid logging full HTML or credentialed remote URLs. |
2 changes: 1 addition & 1 deletion docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ No breaking application code changes are required.

- **Consumers:** `composer update nowo-tech/html-to-word-bundle` and clear cache as usual.
- **Optional:** set `images.remote_timeout` (seconds, default `10`) under a profile’s `images` section if you need a different remote download timeout — see [CONFIGURATION.md](CONFIGURATION.md).
- **Security:** when `images.resolve_remote` is enabled, keep a tight `remote_host_allowlist` (empty allowlist remains permissive) — see [SECURITY.md](SECURITY.md).
- **Security:** when `images.resolve_remote` is enabled, set an explicit `remote_host_allowlist` (empty allowlist is deny-all via `RemoteImageHostPolicy`) — see [SECURITY.md](SECURITY.md).
- **Maintainers:** coverage gate is now **100%** lines; demo FrankenPHP image uses PHP **8.5** with optional `FRANKENPHP_MODE`.

## 1.1.3 → 1.1.4
Expand Down
Loading