diff --git a/src/net/http.zig b/src/net/http.zig index 4615621..02f49d0 100644 --- a/src/net/http.zig +++ b/src/net/http.zig @@ -250,7 +250,7 @@ pub const Client = struct { }.f; const uri = std.Uri.parse(url) catch |err| { - log.warn("http {s}: stage=uri_parse err={s} url={s}", .{ @tagName(method), @errorName(err), url }); + log.warn("http {s}: stage=uri_parse err={s} url={f}", .{ @tagName(method), @errorName(err), redactUrl(url) }); return err; }; const ms_uri_parse = stageElapsedMs(&t_stage, self.io); @@ -284,7 +284,7 @@ pub const Client = struct { // TLS handshake. Logging at warn level (rather than debug) // because DNS / connectivity failures are exactly what // operators need to see immediately. - log.warn("http {s}: stage=connect err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=connect err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; defer req.deinit(); @@ -294,24 +294,24 @@ pub const Client = struct { var send_buf: [4096]u8 = undefined; req.transfer_encoding = .{ .content_length = payload.len }; var bw = req.sendBodyUnflushed(&send_buf) catch |err| { - log.warn("http {s}: stage=send_body_open err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=send_body_open err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; bw.writer.writeAll(payload) catch |err| { - log.warn("http {s}: stage=send_body_write err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=send_body_write err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; bw.end() catch |err| { - log.warn("http {s}: stage=send_body_end err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=send_body_end err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; req.connection.?.flush() catch |err| { - log.warn("http {s}: stage=send_body_flush err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=send_body_flush err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; } else { req.sendBodiless() catch |err| { - log.warn("http {s}: stage=send_bodiless err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=send_bodiless err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; } @@ -320,7 +320,7 @@ pub const Client = struct { // Matches the default redirect capacity in std.http.Client.fetch. var redirect_buffer: [8 * 1024]u8 = undefined; var response = req.receiveHead(&redirect_buffer) catch |err| { - log.warn("http {s}: stage=receive_head err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=receive_head err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; const ms_receive_head = stageElapsedMs(&t_stage, self.io); @@ -359,7 +359,7 @@ pub const Client = struct { var decompress_buffer: [64 * 1024]u8 = undefined; const reader = response.readerDecompressing(&transfer_buffer, &decompress, &decompress_buffer); _ = reader.streamRemaining(&aw.writer) catch |err| { - log.warn("http {s}: stage=stream_body err={s} elapsed_ms={d} url={s}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), url }); + log.warn("http {s}: stage=stream_body err={s} elapsed_ms={d} url={f}", .{ @tagName(method), @errorName(err), stageElapsedMs(&t_stage, self.io), redactUrl(url) }); return err; }; const ms_body = stageElapsedMs(&t_stage, self.io); @@ -368,7 +368,7 @@ pub const Client = struct { const total_ms = @divTrunc(std.Io.Timestamp.now(self.io, .awake).nanoseconds - t_start, std.time.ns_per_ms); log.debug( - "http {s}: ok status={d} bytes={d} total_ms={d} (uri_parse={d} connect={d} send={d} receive_head={d} body={d}) url={s}", + "http {s}: ok status={d} bytes={d} total_ms={d} (uri_parse={d} connect={d} send={d} receive_head={d} body={d}) url={f}", .{ @tagName(method), @intFromEnum(response.head.status), @@ -379,7 +379,7 @@ pub const Client = struct { ms_send, ms_receive_head, ms_body, - url, + redactUrl(url), }, ); @@ -443,6 +443,117 @@ pub const Client = struct { } }; +// ── URL redaction for logs ─────────────────────────────────── +// +// Providers authenticate by query parameter, so a raw request URL is a +// credential. Every log line here used to print `url={s}` verbatim - +// including at `warn` level, which reaches production (ReleaseSafe +// defaults to `.info`). A single upstream 5xx was therefore enough to +// write a live API key into a cron log. Log through `redactUrl` and +// `{f}` instead; never interpolate a URL with `{s}`. + +/// Placeholder substituted for a credential value. +const redacted_marker = "REDACTED"; + +/// Query-parameter names whose values are credentials. +/// +/// Covers every in-tree provider (`token` for Tiingo, `apikey` for FMP +/// and TwelveData, `apiKey` for Polygon) plus the common variants a +/// future provider is likely to use. Matched case-insensitively. +/// +/// **Adding a provider that authenticates by query parameter?** Add its +/// parameter name here. `looksLikeCredential` is a backstop for the +/// case where someone forgets, not a substitute for this list. +const credential_params = [_][]const u8{ + "token", + "apikey", + "api_key", + "apitoken", + "api_token", + "key", + "access_token", + "auth", + "secret", + "password", +}; + +fn isCredentialParam(name: []const u8) bool { + for (credential_params) |candidate| { + if (std.ascii.eqlIgnoreCase(name, candidate)) return true; + } + return false; +} + +/// Backstop heuristic: does this value look like an opaque secret? +/// +/// True for a long run of credential-alphabet bytes and nothing else. +/// Every legitimate query value these providers send is either short +/// (symbols, intervals, booleans, CIKs, CUSIPs - all under 20 bytes) or +/// contains punctuation the alphabet below excludes (dates carry `-` +/// but are 10 bytes; symbol lists carry `,`). Real keys are 32-40 bytes +/// of hex or base62. +/// +/// Deliberately biased toward over-redaction: a redacted debug line is +/// an inconvenience, a leaked key is an incident. +fn looksLikeCredential(value: []const u8) bool { + if (value.len < 20) return false; + for (value) |c| { + switch (c) { + 'A'...'Z', 'a'...'z', '0'...'9', '_', '-' => {}, + else => return false, + } + } + return true; +} + +/// A URL wrapped for safe logging. Render with `{f}`. +pub const RedactedUrl = struct { + url: []const u8, + + pub fn format(self: RedactedUrl, w: *std.Io.Writer) std.Io.Writer.Error!void { + // Everything up to '?' is scheme/host/path - no credentials. + const q = std.mem.indexOfScalar(u8, self.url, '?') orelse { + try w.writeAll(self.url); + return; + }; + try w.writeAll(self.url[0 .. q + 1]); + + // `buildUrl` percent-encodes '&' and '=' out of values, so + // splitting on them is unambiguous. Polygon hand-rolls its + // `apiKey=` suffix but uses the same delimiters. + var first = true; + var it = std.mem.splitScalar(u8, self.url[q + 1 ..], '&'); + while (it.next()) |pair| { + if (!first) try w.writeByte('&'); + first = false; + + const eq = std.mem.indexOfScalar(u8, pair, '=') orelse { + try w.writeAll(pair); + continue; + }; + const name = pair[0..eq]; + const value = pair[eq + 1 ..]; + + try w.writeAll(name); + try w.writeByte('='); + if (isCredentialParam(name) or looksLikeCredential(value)) { + try w.writeAll(redacted_marker); + } else { + try w.writeAll(value); + } + } + } +}; + +/// Wrap a URL so `{f}` renders it with credential values elided. +/// +/// Use this at every log site that reports a URL. The parameter name +/// and the non-secret values survive, so an operator can still see +/// which symbol and date range failed. +pub fn redactUrl(url: []const u8) RedactedUrl { + return .{ .url = url }; +} + /// Build a URL with query parameters. Values are percent-encoded per RFC 3986. pub fn buildUrl( allocator: std.mem.Allocator, @@ -636,3 +747,105 @@ test "Response.verifyIntegrity: mismatched sha256 returns mismatch" { else => try std.testing.expect(false), } } + +// ── redactUrl ──────────────────────────────────────────────── +// +// Regression suite for a credential leak: every `url=` log line - nine +// of them at `warn`, which reaches production under ReleaseSafe - used +// to print the request URL verbatim, so any upstream failure wrote a +// live API key into the log. + +/// Render through the `{f}` path the log statements use. +fn expectRedacted(expected: []const u8, url: []const u8) !void { + var buf: [512]u8 = undefined; + const got = try std.fmt.bufPrint(&buf, "{f}", .{redactUrl(url)}); + try std.testing.expectEqualStrings(expected, got); +} + +test "redactUrl: hides every in-tree provider's credential parameter" { + // Tiingo (`token`) - the key that actually leaked. + try expectRedacted( + "https://api.tiingo.com/tiingo/daily/SPY/prices?startDate=2026-08-07&endDate=2026-08-17&token=REDACTED", + "https://api.tiingo.com/tiingo/daily/SPY/prices?startDate=2026-08-07&endDate=2026-08-17&token=0123456789abcdef0123456789abcdef01234567", + ); + // FMP and TwelveData (`apikey`). + try expectRedacted( + "https://financialmodelingprep.com/stable/earnings?symbol=SMPL&apikey=REDACTED", + "https://financialmodelingprep.com/stable/earnings?symbol=SMPL&apikey=cafebabecafebabecafebabecafebabe", + ); + // Polygon (`apiKey`) - different case, and hand-rolled rather than + // built by `buildUrl`. + try expectRedacted( + "https://api.polygon.io/v3/reference/dividends?ticker=SMPL&apiKey=REDACTED", + "https://api.polygon.io/v3/reference/dividends?ticker=SMPL&apiKey=deadbeefdeadbeefdeadbeefdeadbeef", + ); +} + +test "redactUrl: keeps the diagnostically useful parts" { + // The point of these log lines is "which request failed?". Symbol, + // dates and interval must survive so they stay actionable. + try expectRedacted( + "https://api.example.com/v1/bars?symbol=SMPL&interval=1d&startDate=2026-08-07&adjusted=true&token=REDACTED", + "https://api.example.com/v1/bars?symbol=SMPL&interval=1d&startDate=2026-08-07&adjusted=true&token=0123456789abcdef0123456789abcdef01234567", + ); + // A comma-separated symbol list is long but is not a credential. + try expectRedacted( + "https://api.example.com/quotes?symbols=SMPLA,SMPLB,SMPLC,SMPLD,SMPLE", + "https://api.example.com/quotes?symbols=SMPLA,SMPLB,SMPLC,SMPLD,SMPLE", + ); +} + +test "redactUrl: passes through URLs with no query string" { + try expectRedacted("https://zfin.example.org/SPY/diagnostics", "https://zfin.example.org/SPY/diagnostics"); + try expectRedacted("https://zfin.example.org/SPY/candles?", "https://zfin.example.org/SPY/candles?"); +} + +test "redactUrl: tolerates malformed query segments" { + // A bare flag with no '=' must not be dropped or mangled. + try expectRedacted( + "https://api.example.com/x?flag&symbol=SMPL&token=REDACTED", + "https://api.example.com/x?flag&symbol=SMPL&token=0123456789abcdef0123456789abcdef01234567", + ); + // An empty credential value stays redacted rather than revealing + // that the key was absent. + try expectRedacted("https://api.example.com/x?token=REDACTED", "https://api.example.com/x?token="); +} + +test "redactUrl: heuristic backstops an unlisted parameter name" { + // If a future provider authenticates with a name nobody added to + // `credential_params`, an opaque 32-byte value still gets caught. + try expectRedacted( + "https://api.example.com/x?symbol=SMPL&sessionCredential=REDACTED", + "https://api.example.com/x?symbol=SMPL&sessionCredential=cafebabecafebabecafebabecafebabe", + ); +} + +test "looksLikeCredential: separates opaque secrets from real query values" { + // Secrets: 32-byte hex, 40-byte hex, base62-with-dash-and-underscore. + try std.testing.expect(looksLikeCredential("cafebabecafebabecafebabecafebabe")); + try std.testing.expect(looksLikeCredential("0123456789abcdef0123456789abcdef01234567")); + try std.testing.expect(looksLikeCredential("ya29_A0ARrdaM-abcdefghijklmnop")); + + // Everything these providers legitimately send is either short... + try std.testing.expect(!looksLikeCredential("SMPL")); + try std.testing.expect(!looksLikeCredential("2026-08-07")); + try std.testing.expect(!looksLikeCredential("1d")); + try std.testing.expect(!looksLikeCredential("true")); + try std.testing.expect(!looksLikeCredential("0000320193")); + // ...or long but punctuated in ways a key never is. + try std.testing.expect(!looksLikeCredential("SMPLA,SMPLB,SMPLC,SMPLD,SMPLE")); + try std.testing.expect(!looksLikeCredential("2026-08-07T00:00:00.000Z")); +} + +test "isCredentialParam: case-insensitive across provider spellings" { + try std.testing.expect(isCredentialParam("token")); + try std.testing.expect(isCredentialParam("apikey")); + try std.testing.expect(isCredentialParam("apiKey")); + try std.testing.expect(isCredentialParam("APIKEY")); + try std.testing.expect(isCredentialParam("api_key")); + try std.testing.expect(isCredentialParam("access_token")); + + try std.testing.expect(!isCredentialParam("symbol")); + try std.testing.expect(!isCredentialParam("ticker")); + try std.testing.expect(!isCredentialParam("startDate")); +}