Reviewed by Hermes Agent — Request changes (one scope issue; the security work itself is solid) 🟢🔴

I checked this out locally and ran the new tests against current main. All five hardening changes verified passing:

• test_v4a_move_file_header_rejects_traversal ✓
• test_acp_resource_link_blocks_secret_env_files ✓
• test_health_detailed_requires_auth_when_key_configured ✓
• test_rejects_assistant_final_message ✓
• test_rejects_private_image_url ✓

(plus the related auth-guard tests in test_api_server.py — 17 passed total.)

The security changes are well done:

• V4A Move guard (file_tools.py) — regex now captures *** Move File: src -> dst with both paths in separate groups, filters empties, and loops all paths through has_traversal_component. Correct and complete.
• ACP resource reads (acp_adapter/server.py) — routes reads through get_read_block_error() (verified the helper exists in agent/file_safety.py), with graceful OSError handling and a sensible Windows path fix. Blocks .env/SSH keys/creds as claimed.
• API SSRF + auth (api_server.py) — is_safe_url() guard on image URLs (helper confirmed in tools/url_safety.py), /health/detailed now gated behind _check_auth, and the role-alternation guard rejecting a non-user final message is a genuinely good catch (prevents client-supplied assistant text becoming fresh instructions).
• Discord allowed_mentions — added to all three REST send paths via _build_allowed_mentions_payload(), denying @everyone/@roles by default.
• Webhook idempotency scoped to route:delivery_id:body_sha256 — sound.

The one blocker — an unrelated change that needs to come out:

pyproject.toml flips the global pytest config:

-addopts = "-m 'not integration' --timeout=30 --timeout-method=signal"
+addopts = "-m 'not integration' --timeout=60 --timeout-method=thread"

This isn't security hardening — it's a repo-wide test-infra change (doubles the per-test timeout and switches signal→thread) buried in a security PR, and it also deletes the explanatory comment block documenting why signal is used inside the per-file subprocess runner (scripts/run_tests_parallel.py). The PR description doesn't mention it at all.

Please revert the pyproject.toml hunk so this PR is purely the security hardening. If the timeout/method change is intentional and needed, it deserves its own PR with rationale (why thread over signal, why 60s) so it can be evaluated on its own merits.

Drop that one hunk and I'm happy to approve — the actual hardening is clean, well-scoped per file, and properly tested.

Needs rework.

I stopped at the early gates for the current branch state. Current GitHub main is d1383a6b1450c6c139720b1b01f8b99cc130453f, PR base is b5a457c033035e8dcd203745e68be16b50c2390e, and PR head is d17e155741301c9844ab0bfcac5d29f831ea756a.

Validation:

• git rev-list --left-right --count upstream/main...refs/remotes/upstream/pr/41754 returned 202 175.
• git merge-tree --write-tree upstream/main refs/remotes/upstream/pr/41754 exited non-zero and reported conflicts across many unrelated areas, including desktop files, gateway platform files, hermes_cli/*, plugin files, tests, tools/approval.py, tui_gateway/server.py, and docs.
• git diff --check upstream/main...refs/remotes/upstream/pr/41754 fails with tests/hermes_cli/test_gui_uninstall.py:348: new blank line at EOF.
• git diff --stat upstream/main...refs/remotes/upstream/pr/41754 now spans 439 files with about 28k insertions, including desktop UI/i18n, plugins, docs, CLI, gateway, and many unrelated test files outside the advertised V4A/ACP/API/webhook/Discord hardening scope.

The individual security ideas may still be valuable, but this branch is not in a reviewable or mergeable shape against current main. Please rebase/rescope it to the listed hardening changes only and remove the unrelated repo-wide drift before requesting another review.

Signed: GPT-5.5-xhigh in Codex

Recommendation: request changes

I reviewed this against current GitHub main fa7f24e8980367c2ca849eb99e1eb2331c7d3699, PR base 3e74f75e41ecd5a3b937d692ba7dcffbf77304f6, and PR head 6f70a0f7efbf95afe7dd12495d6318496e44806c.

Validation:

• git merge-tree --write-tree upstream/main 6f70a0f7efbf95afe7dd12495d6318496e44806c: passed, produced tree 7a4dab0a6719ad6a605479e9e91b6fe3ae5c679a.
• git diff --check upstream/main...6f70a0f7efbf95afe7dd12495d6318496e44806c: passed.
• pytest -q tests/tools/test_file_tools.py::TestSensitivePathCheck::test_v4a_move_file_header_rejects_traversal tests/acp_adapter/test_acp_images.py::test_acp_resource_link_blocks_secret_env_files tests/gateway/test_api_server.py::TestHealthDetailedEndpoint::test_health_detailed_requires_auth_when_key_configured tests/gateway/test_api_server.py::TestChatCompletionsEndpoint::test_rejects_assistant_final_message tests/gateway/test_api_server.py::TestChatCompletionsEndpoint::test_rejects_private_image_url tests/gateway/test_discord_allowed_mentions.py -p no:cacheprovider: passed, 24 passed and 3 aiohttp app-key warnings.
• pytest -q tests/gateway/test_webhook_adapter.py::TestIdempotency -p no:cacheprovider: failed, 1 failed and 2 passed.

Finding:
The webhook idempotency test suite is broken by the new scoped _seen_deliveries key. TestIdempotency.test_expired_delivery_id_allows_reprocess still backdates adapter._seen_deliveries["delivery-456"], but the implementation now stores keys as route:delivery_id:body_sha256. The actual fresh scoped key remains in the cache, so the second request is still treated as a duplicate and the test fails with AssertionError: assert 200 == 202.

Please update the idempotency test and any helper code so TTL expiry is validated against the new key shape.

Signed: GPT-5.5-xhigh in Codex