mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 07:30:11 +00:00
fix: HTTP webhook secret transmitted in request body rather than via header, docs inconsistency and security concern (#1162)
Implement industry-standard HMAC-SHA256 header-based webhook authentication to resolve issue #722. The X-Hub-Signature-256 header follows GitHub's webhook security model, replacing the non-standard X-IronClaw-Signature header. **Changes:** - Rename HTTP webhook signature header from X-IronClaw-Signature to X-Hub-Signature-256 - X-Hub-Signature-256 is the standard used by GitHub, Stripe, and other webhook providers - HMAC-SHA256 signatures continue to use sha256=<hex> format - Body 'secret' field remains supported as deprecated fallback for backward compatibility - All error messages and documentation updated to reflect new header name **Security impact:** - Signatures verified via HTTP header instead of request body - Signature visible in Authorization header only, not logged in request body - Follows industry best practices for webhook authentication - Fail-closed policy: rejects requests without authentication **Backward compatibility:** - Requests without X-Hub-Signature-256 header fall back to 'secret' field in body (with deprecation warning) - Deprecation path: migrate to header-based auth, body field support will be removed in a future release **Test coverage:** Unit tests (20 tests in src/channels/http.rs): - 6 header-based auth tests (valid/invalid/malformed signatures, header encoding) - 2 backward compatibility tests (deprecated body secret fallback) - 3 error handling tests (missing auth, invalid JSON, content-type validation) - 4 signature verification unit tests (valid digest, invalid digest, missing prefix, invalid hex) - 5 advanced tests (concurrency, dynamic updates, header precedence, no deadlocks, runtime clearing) E2E tests (12 tests in tests/e2e/scenarios/test_webhook.py): - Valid HMAC-SHA256 signature acceptance - Invalid/wrong/malformed signature rejection - Header precedence over body secret - Deprecated body secret backward compatibility - Missing auth rejection (fail-closed) - Content-Type validation - Invalid JSON handling - Case-insensitive header lookup - Message queuing and processing - Fixture for running server with HTTP_WEBHOOK_SECRET configured All 3,033 lib tests pass with zero clippy warnings. **Example usage after fix:** BODY='{"content": "hello"}' SECRET="your-webhook-secret" SIG=$(echo -n "$BODY" | openssl dgst -sha256 -hmac "$SECRET" | sed 's/^.* //') curl -X POST http://127.0.0.1:9090/webhook \ -H "Content-Type: application/json" \ -H "X-Hub-Signature-256: sha256=$SIG" \ -d "$BODY" Co-authored-by: Claude Haiku 4.5 <[email protected]>
This commit is contained in:
Nick Pismenkov
co-authored by
Claude Haiku 4.5
Claude Haiku 4.5
parent
c916069dd2
commit
8fb2f70258
@@ -220,6 +220,97 @@ async def ironclaw_server(ironclaw_binary, mock_llm_server, wasm_tools_dir):
|
||||
proc.kill()
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
async def ironclaw_server_with_webhook_secret(ironclaw_binary, mock_llm_server, wasm_tools_dir):
|
||||
"""Start ironclaw with HTTP_WEBHOOK_SECRET configured for webhook tests.
|
||||
|
||||
Yields a dict with:
|
||||
- 'url': base URL of the gateway
|
||||
- 'secret': the webhook secret value
|
||||
"""
|
||||
gateway_port = _find_free_port()
|
||||
webhook_secret = "test-webhook-secret-e2e-12345"
|
||||
env = {
|
||||
# Minimal env: PATH for process spawning, HOME for Rust/cargo defaults
|
||||
"PATH": os.environ.get("PATH", "/usr/bin:/bin"),
|
||||
"HOME": os.environ.get("HOME", "/tmp"),
|
||||
"RUST_LOG": "ironclaw=info",
|
||||
"RUST_BACKTRACE": "1",
|
||||
"GATEWAY_ENABLED": "true",
|
||||
"GATEWAY_HOST": "127.0.0.1",
|
||||
"GATEWAY_PORT": str(gateway_port),
|
||||
"GATEWAY_AUTH_TOKEN": AUTH_TOKEN,
|
||||
"GATEWAY_USER_ID": "e2e-tester",
|
||||
"HTTP_WEBHOOK_SECRET": webhook_secret,
|
||||
"CLI_ENABLED": "false",
|
||||
"LLM_BACKEND": "openai_compatible",
|
||||
"LLM_BASE_URL": mock_llm_server,
|
||||
"LLM_MODEL": "mock-model",
|
||||
"DATABASE_BACKEND": "libsql",
|
||||
"LIBSQL_PATH": os.path.join(_DB_TMPDIR.name, "e2e-webhook.db"),
|
||||
"SANDBOX_ENABLED": "false",
|
||||
"SKILLS_ENABLED": "true",
|
||||
"ROUTINES_ENABLED": "false",
|
||||
"HEARTBEAT_ENABLED": "false",
|
||||
"EMBEDDING_ENABLED": "false",
|
||||
# WASM tool/channel support
|
||||
"WASM_ENABLED": "true",
|
||||
"WASM_TOOLS_DIR": wasm_tools_dir,
|
||||
"WASM_CHANNELS_DIR": _WASM_CHANNELS_TMPDIR.name,
|
||||
# Prevent onboarding wizard from triggering
|
||||
"ONBOARD_COMPLETED": "true",
|
||||
# Force gateway OAuth callback mode (non-loopback URL) and point
|
||||
# token exchange at mock_llm.py so OAuth tests work without Google.
|
||||
"IRONCLAW_OAUTH_CALLBACK_URL": "https://oauth.test.example/oauth/callback",
|
||||
"IRONCLAW_OAUTH_EXCHANGE_URL": mock_llm_server,
|
||||
}
|
||||
# Forward LLVM coverage instrumentation env vars when present
|
||||
COV_ENV_PREFIXES = ("CARGO_LLVM_COV", "LLVM_")
|
||||
COV_ENV_EXTRAS = ("CARGO_ENCODED_RUSTFLAGS", "CARGO_INCREMENTAL")
|
||||
for key, val in os.environ.items():
|
||||
if key.startswith(COV_ENV_PREFIXES) or key in COV_ENV_EXTRAS:
|
||||
env[key] = val
|
||||
proc = await asyncio.create_subprocess_exec(
|
||||
ironclaw_binary, "--no-onboard",
|
||||
stdin=asyncio.subprocess.DEVNULL,
|
||||
stdout=asyncio.subprocess.PIPE,
|
||||
stderr=asyncio.subprocess.PIPE,
|
||||
env=env,
|
||||
)
|
||||
base_url = f"http://127.0.0.1:{gateway_port}"
|
||||
try:
|
||||
await wait_for_ready(f"{base_url}/api/health", timeout=60)
|
||||
yield {
|
||||
"url": base_url,
|
||||
"secret": webhook_secret,
|
||||
}
|
||||
except TimeoutError:
|
||||
# Dump stderr so CI logs show why the server failed to start
|
||||
returncode = proc.returncode
|
||||
stderr_bytes = b""
|
||||
if proc.stderr:
|
||||
try:
|
||||
stderr_bytes = await asyncio.wait_for(proc.stderr.read(8192), timeout=2)
|
||||
except (asyncio.TimeoutError, Exception):
|
||||
pass
|
||||
stderr_text = stderr_bytes.decode("utf-8", errors="replace")
|
||||
proc.kill()
|
||||
pytest.fail(
|
||||
f"ironclaw server with webhook secret failed to start on port {gateway_port} "
|
||||
f"(returncode={returncode}).\nstderr:\n{stderr_text}"
|
||||
)
|
||||
finally:
|
||||
if proc.returncode is None:
|
||||
# Use SIGINT (not SIGTERM) so tokio's ctrl_c handler triggers a
|
||||
# graceful shutdown. This lets the LLVM coverage runtime run its
|
||||
# atexit handler and flush .profraw files for cargo-llvm-cov.
|
||||
proc.send_signal(signal.SIGINT)
|
||||
try:
|
||||
await asyncio.wait_for(proc.wait(), timeout=10)
|
||||
except asyncio.TimeoutError:
|
||||
proc.kill()
|
||||
|
||||
|
||||
@pytest.fixture(scope="session")
|
||||
async def browser(ironclaw_server):
|
||||
"""Session-scoped Playwright browser instance.
|
||||
|
||||
@@ -0,0 +1,340 @@
|
||||
"""HTTP webhook authentication tests with HMAC-SHA256 signatures."""
|
||||
|
||||
import hashlib
|
||||
import hmac
|
||||
import json
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from helpers import AUTH_TOKEN
|
||||
|
||||
|
||||
def compute_signature(secret: str, body: bytes) -> str:
|
||||
"""Compute X-Hub-Signature-256 HMAC-SHA256 signature."""
|
||||
mac = hmac.new(secret.encode(), body, hashlib.sha256)
|
||||
return f"sha256={mac.hexdigest()}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_requires_http_webhook_secret_configured(ironclaw_server):
|
||||
"""
|
||||
Webhook endpoint rejects requests when HTTP_WEBHOOK_SECRET is not configured.
|
||||
This tests the fail-closed security posture.
|
||||
"""
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
async with httpx.AsyncClient() as client:
|
||||
# When no webhook secret is configured on the server, all requests fail
|
||||
r = await client.post(
|
||||
f"{ironclaw_server}/webhook",
|
||||
json={"content": "test message"},
|
||||
headers=headers,
|
||||
)
|
||||
# Server should reject with 503 Service Unavailable (fail closed)
|
||||
assert r.status_code in (401, 503)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_hmac_signature_valid(ironclaw_server_with_webhook_secret):
|
||||
"""Valid X-Hub-Signature-256 HMAC signature is accepted."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello from webhook"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 200, f"Expected 200, got {r.status_code}: {r.text}"
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_invalid_hmac_signature_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Invalid X-Hub-Signature-256 signature is rejected with 401."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
invalid_signature = "sha256=0000000000000000000000000000000000000000000000000000000000000000"
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": invalid_signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401, f"Expected 401, got {r.status_code}"
|
||||
resp = r.json()
|
||||
assert resp["status"] == "error"
|
||||
assert "Invalid webhook signature" in resp.get("response", "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_wrong_secret_rejected(ironclaw_server_with_webhook_secret):
|
||||
"""Signature computed with wrong secret is rejected."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
# Compute signature with wrong secret
|
||||
wrong_signature = compute_signature("wrong-secret", body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": wrong_signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
resp = r.json()
|
||||
assert resp["status"] == "error"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_malformed_signature_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Malformed X-Hub-Signature-256 header is rejected."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
# Missing sha256= prefix
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": "deadbeef",
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_missing_signature_header_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Missing X-Hub-Signature-256 header is rejected when no body secret provided."""
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
# No X-Hub-Signature-256 header and no body secret
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401
|
||||
resp = r.json()
|
||||
assert "Webhook authentication required" in resp.get("response", "")
|
||||
assert "X-Hub-Signature-256" in resp.get("response", "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_deprecated_body_secret_still_works(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""
|
||||
Deprecated: body 'secret' field still works for backward compatibility.
|
||||
This test ensures we don't break existing clients during the migration period.
|
||||
"""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
# Old-style request with secret in body
|
||||
body_data = {"content": "hello", "secret": secret}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
},
|
||||
)
|
||||
# Should succeed (backward compatibility)
|
||||
assert r.status_code == 200, f"Expected 200, got {r.status_code}: {r.text}"
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_header_takes_precedence_over_body_secret(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""
|
||||
When both X-Hub-Signature-256 header and body secret are provided,
|
||||
header takes precedence.
|
||||
"""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello", "secret": "wrong-secret-in-body"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
# Compute signature with correct secret
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
# Should succeed because header signature is valid (takes precedence)
|
||||
assert r.status_code == 200
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_case_insensitive_header_lookup(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""HTTP headers are case-insensitive. Test with different cases."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
# Try with lowercase
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"x-hub-signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_wrong_content_type_rejected(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Webhook only accepts application/json Content-Type."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_data = {"content": "hello"}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "text/plain",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 415 # Unsupported Media Type
|
||||
resp = r.json()
|
||||
assert "application/json" in resp.get("response", "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_invalid_json_rejected(ironclaw_server_with_webhook_secret):
|
||||
"""Invalid JSON in body is rejected."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
body_bytes = b"not valid json"
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 401 or r.status_code == 400
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_webhook_message_queued_for_processing(
|
||||
ironclaw_server_with_webhook_secret,
|
||||
):
|
||||
"""Message via webhook is queued and can be retrieved."""
|
||||
secret = ironclaw_server_with_webhook_secret["secret"]
|
||||
base_url = ironclaw_server_with_webhook_secret["url"]
|
||||
|
||||
headers = {"Authorization": f"Bearer {AUTH_TOKEN}"}
|
||||
test_message = "webhook test message 12345"
|
||||
body_data = {"content": test_message}
|
||||
body_bytes = json.dumps(body_data).encode()
|
||||
signature = compute_signature(secret, body_bytes)
|
||||
|
||||
async with httpx.AsyncClient() as client:
|
||||
r = await client.post(
|
||||
f"{base_url}/webhook",
|
||||
content=body_bytes,
|
||||
headers={
|
||||
**headers,
|
||||
"Content-Type": "application/json",
|
||||
"X-Hub-Signature-256": signature,
|
||||
},
|
||||
)
|
||||
assert r.status_code == 200
|
||||
resp = r.json()
|
||||
assert resp["status"] == "ok"
|
||||
# Message ID should be present
|
||||
assert "message_id" in resp
|
||||
assert resp["message_id"] != "00000000-0000-0000-0000-000000000000"
|
||||
Reference in New Issue
Block a user