Skip to content

Commit 846ebce

Browse files
authored
fix(http): send default User-Agent; harden flaky 06 http/ssrf e2e (#270)
The durable HTTP client (reqwest) previously sent no User-Agent header. Some hosts reject anonymous requests: httpbingo.org (fly.io) returns HTTP 402 to any request without a User-Agent, which made the redirect test assert 3xx but receive 402, and silently let the other httpbingo tests "pass" on a 402 instead of a real success response. Product fix: - build_client() now sets a default User-Agent of "pg_durable/<version>". Nodes may still override it via an explicit User-Agent header. Test hardening (06_http_and_ssrf.sql): - Move the redirect test (Test 8) off httpbin.org (frequently unavailable; the dominant CI flake) onto httpbingo.org/status/302, inheriting the default 30s timeout. - Remove the residual-PUBLIC-grant permission test: it only exercised stock PostgreSQL grant semantics (a PUBLIC grant survives an additive per-role grant) and asserted nothing pg_durable-specific. Renumber the superuser bypass test accordingly. Harness fix (scripts/test-e2e-local.sh): - The repeat-run re-grant path (grant_e2e_df_usage) used the default include_http => false, so on runs >= 2 (after the extension is dropped and recreated) df_e2e_user lost HTTP access and 06 failed at the first df.http() call. Pass include_http => true to mirror 00_setup_playground, fixing `./scripts/test-e2e-local.sh 06_http_and_ssrf N`. Verified: 06_http_and_ssrf now passes 3/3 in repeat mode against a reused database. A separate, pre-existing BGW cold-start "pending" timeout flake on the first instance remains out of scope (tracked for a retry harness).
1 parent 20cef43 commit 846ebce

3 files changed

Lines changed: 14 additions & 32 deletions

File tree

scripts/test-e2e-local.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ show_version_once() {
363363

364364
grant_e2e_df_usage() {
365365
"$PSQL" -h localhost -p "$PG_PORT" -U "$PG_USER" -d "$PG_DB" \
366-
-c "SELECT df.grant_usage('$E2E_USER');" >/dev/null 2>&1
366+
-c "SELECT df.grant_usage('$E2E_USER', include_http => true);" >/dev/null 2>&1
367367
}
368368

369369
ensure_e2e_role() {

src/activities/execute_http.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,13 +49,18 @@ async fn check_http_privilege(pool: &PgPool, submitted_by: &str) -> Result<(), S
4949

5050
/// Build a reqwest Client with optional SSRF-safe DNS resolver.
5151
///
52+
/// A default `User-Agent` is set so requests are not anonymous: some endpoints
53+
/// (e.g. fly.io-hosted services) reject requests that omit it. Nodes may still
54+
/// override it via an explicit `User-Agent` header.
55+
///
5256
/// Redirects are disabled to prevent redirect-based SSRF bypasses: an attacker
5357
/// could host a 302 redirecting to `http://169.254.169.254/...`, and reqwest
5458
/// would follow it without calling our DNS resolver (since the target is an IP
5559
/// literal).
5660
fn build_client(timeout: Duration) -> Result<reqwest::Client, String> {
5761
let builder = reqwest::Client::builder()
5862
.timeout(timeout)
63+
.user_agent(concat!("pg_durable/", env!("CARGO_PKG_VERSION")))
5964
.redirect(reqwest::redirect::Policy::none());
6065

6166
// Inject the SSRF-safe DNS resolver unless http-allow-all removes all guards.

tests/e2e/sql/06_http_and_ssrf.sql

Lines changed: 8 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -580,10 +580,12 @@ END $$;
580580
DROP TABLE _test_ssrf7;
581581

582582
-- Test 8: Redirects are not followed
583+
-- Uses httpbingo.org (maintained Go reimplementation) rather than httpbin.org,
584+
-- which is frequently unavailable and was the dominant source of CI flakiness.
583585
CREATE TEMP TABLE _test_ssrf8 (instance_id TEXT);
584586

585587
INSERT INTO _test_ssrf8 SELECT df.start(
586-
df.http('https://httpbin.org/status/302', 'GET', NULL, NULL, 10) |=> 'response'
588+
df.http('https://httpbingo.org/status/302', 'GET') |=> 'response'
587589
~> 'SELECT ($response::jsonb->>''status'')::int AS status_code',
588590
'test-ssrf-redirect-blocked'
589591
);
@@ -937,34 +939,9 @@ END $$;
937939

938940
DROP TABLE _test_http_priv4;
939941

940-
-- Permission Test 5: grant_usage(false) does NOT grant df.http, but residual PUBLIC grant persists
941-
GRANT EXECUTE ON FUNCTION df.http(text, text, text, jsonb, integer) TO PUBLIC;
942-
943-
-- grant_usage is purely additive — it does not revoke df.http or warn about
944-
-- residual PUBLIC grants. The caller is responsible for revoking separately.
945-
946-
DO $$
947-
BEGIN
948-
BEGIN
949-
PERFORM df.grant_usage('http_priv_bob', include_http => false);
950-
EXCEPTION WHEN OTHERS THEN
951-
RAISE EXCEPTION 'TEST FAILED: grant_usage raised an unexpected exception: %', SQLERRM;
952-
END;
953-
-- grant_usage(include_http => false) does not grant df.http, but the
954-
-- PUBLIC grant above still gives http_priv_bob effective access.
955-
IF NOT has_function_privilege('http_priv_bob'::regrole,
956-
'df.http(text, text, text, jsonb, integer)', 'EXECUTE') THEN
957-
RAISE EXCEPTION 'TEST FAILED: expected http_priv_bob to still have effective HTTP access via PUBLIC grant';
958-
END IF;
959-
RAISE NOTICE 'TEST PASSED: grant_usage_warns_on_residual_public_grant';
960-
END $$;
961-
962-
-- Restore: revoke the PUBLIC grant again so subsequent tests are unaffected
963-
REVOKE EXECUTE ON FUNCTION df.http(text, text, text, jsonb, integer) FROM PUBLIC;
964-
965-
-- Permission Test 6: Superuser always passes the execution-time privilege check
966-
CREATE TEMP TABLE _test_http_priv6 (instance_id TEXT);
967-
INSERT INTO _test_http_priv6
942+
-- Permission Test 5: Superuser always passes the execution-time privilege check
943+
CREATE TEMP TABLE _test_http_priv5 (instance_id TEXT);
944+
INSERT INTO _test_http_priv5
968945
SELECT df.start(
969946
'{"node_type":"HTTP","query":"{\"url\":\"https://api.github.com/\",\"method\":\"GET\",\"body\":null,\"headers\":null,\"timeout_seconds\":5}"}',
970947
'test-http-superuser'
@@ -976,7 +953,7 @@ DECLARE
976953
status TEXT;
977954
node_result TEXT;
978955
BEGIN
979-
SELECT instance_id INTO inst_id FROM _test_http_priv6;
956+
SELECT instance_id INTO inst_id FROM _test_http_priv5;
980957

981958
SELECT df.await_instance(inst_id, 30) INTO status;
982959

@@ -995,7 +972,7 @@ BEGIN
995972
RAISE NOTICE 'TEST PASSED: superuser_bypasses_http_privilege_check';
996973
END $$;
997974

998-
DROP TABLE _test_http_priv6;
975+
DROP TABLE _test_http_priv5;
999976

1000977
-- --- Permission Test Cleanup ---
1001978
DO $cleanup$

0 commit comments

Comments
 (0)