Keep unanswered requests on disk, out of Sentry
The request bodies send_default_pii was sending are not sensitive - opaque
contact UIDs on a read-only server, and tailnet IPs. It is off because
log/unhandled now keeps the same bodies in better shape: 404s and 5xx are
written in the fixture layout, deduplicated by request digest, so a client
asking for something unimplemented leaves behind a case that can be
promoted straight into the test suite. A write path would also put whole
vCards in those bodies.
Assisted-by: Claude Opus 5 via Claude Code
diff --git a/AGENTS.md b/AGENTS.md
index a39c72a..7620b00 100644
--- a/AGENTS.md
+++ b/AGENTS.md
@@ -64,6 +64,12 @@ before regenerating.
a bare `curl` against `rake dev` is refused until you pass one. The
security of that rests on the app being reachable only through
`tailscale serve` — never bind it to anything but localhost.
+- Unanswered requests (404s and 5xx) are written to `log/unhandled` in the
+ fixture layout. When implementing something a client asked for, look there
+ first — and strip the identifying headers before promoting a capture into
+ `test/fixtures`.
+- `config.ru` must stay ASCII-only; a test enforces it, because boot crashes
+ under a C locale otherwise. Watch for em dashes in comments.
- `RUBYOPT=--enable-frozen-string-literal` is set in `.ramekin/config.kdl`.
String literals are frozen; mutating one raises.
- Application code reads configuration through `ProTacts.config` only; add
diff --git a/README.md b/README.md
index b8f9812..9b6a898 100644
--- a/README.md
+++ b/README.md
@@ -42,6 +42,13 @@ while the app is reachable through serve alone — bind it to localhost.
Tailscale documents two cases that carry no identity and so cannot get in:
Funnel traffic, which is public, and traffic from tagged devices.
+Requests the server cannot answer — a 404, or a crash — are kept under
+`log/unhandled`, one directory per distinct request, in the same layout as
+`test/fixtures/macos-exchange`. A client asking for something unimplemented
+therefore leaves behind enough to implement it, and the capture can be
+promoted to a fixture by copying it and stripping the identifying headers.
+Sentry gets the event without the request body.
+
## The minimal set macOS Contacts needs
The responses are the verified minimum for macOS 26.5.1 Contacts, found
diff --git a/config.ru b/config.ru
index 8fa4d0f..1c6a4d1 100644
--- a/config.ru
+++ b/config.ru
@@ -22,9 +22,14 @@ Sentry.init do |sentry|
# Get breadcrumbs from logs
sentry.breadcrumbs_logger = [:sentry_logger, :http_logger]
- # Add data like request headers and IP for users, if applicable;
- # see https://docs.sentry.io/platforms/ruby/data-management/data-collected/ for more info
- sentry.send_default_pii = true
+ # Off keeps the request body, query string, cookies, and client IP out of
+ # events. None of that is sensitive today: a read-only server's request
+ # bodies carry opaque contact UIDs, not card content, and the IPs are
+ # tailnet addresses. It is off because log/unhandled keeps the same bodies
+ # in better shape (see ProTacts::UnhandledRequests), and because a write
+ # path would put whole vCards in them. Headers still go, including
+ # Tailscale-User-Login, which says who hit the 404.
+ sentry.send_default_pii = false
# Trace all the things!
sentry.traces_sample_rate = 1.0
diff --git a/lib/pro_tacts/config.rb b/lib/pro_tacts/config.rb
index e60c5bf..ad22d55 100644
--- a/lib/pro_tacts/config.rb
+++ b/lib/pro_tacts/config.rb
@@ -39,6 +39,13 @@ module ProTacts
data_dir / "contacts"
end
+ # Where requests the app could not answer are kept, one directory per
+ # distinct request. Under log/ because it holds request data and is not
+ # meant to be committed. See ProTacts::UnhandledRequests.
+ def unhandled_dir
+ Pathname.new(@env.fetch("PRO_TACTS_UNHANDLED_DIR", "log/unhandled"))
+ end
+
# Where the debug logger writes. A path, overridable with
# PRO_TACTS_DEBUG_LOG; "stderr" keeps it on the process's stderr.
def debug_log_path
diff --git a/lib/pro_tacts/unhandled_requests.rb b/lib/pro_tacts/unhandled_requests.rb
new file mode 100644
index 0000000..1cc24b1
--- /dev/null
+++ b/lib/pro_tacts/unhandled_requests.rb
@@ -0,0 +1,115 @@
+require "digest"
+require "fileutils"
+require "pathname"
+
+module ProTacts
+ # Keeps a full copy of the requests the app could not answer, so a client
+ # asking for something unimplemented leaves behind enough to implement it.
+ #
+ # Each capture is a directory holding "request" and "response", the same
+ # layout and format as test/fixtures/macos-exchange, so promoting one to a
+ # fixture is a copy. Strip the identifying headers when you do — captures
+ # keep every header, fixtures do not.
+ #
+ # This is the local counterpart to the Sentry reporting in config.ru, which
+ # no longer sends request bodies.
+ class UnhandledRequests
+ # 404 is the missing-functionality signal: a client asked for something
+ # this server does not route. 5xx is kept because Sentry now reports
+ # those without a body, and a crash is hard to read without one.
+ def self.capture?(status)
+ status == 404 || status >= 500
+ end
+
+ def initialize(app, directory:)
+ @app = app
+ @directory = Pathname.new(directory)
+ end
+
+ def call(env)
+ status, headers, body = @app.call(env)
+ return [status, headers, body] unless self.class.capture?(status)
+
+ parts = []
+ body.each { parts << it }
+ body.close if body.respond_to?(:close)
+
+ capture(env, status, headers, parts.join)
+
+ [status, headers, parts]
+ end
+
+ private
+
+ def capture(env, status, headers, body)
+ target = @directory / name_for(env)
+
+ # The directory name carries a digest of the request, so an already
+ # captured request is one a client is repeating — recording it again
+ # would just grow the directory without adding anything.
+ return if target.exist?
+
+ FileUtils.mkdir_p(target)
+ (target / "request").write(render_request(env))
+ (target / "response").write(render_response(status, headers, body))
+ rescue SystemCallError => e
+ # A failed capture must not turn a 404 into a 500.
+ warn "pro-tacts: could not record unhandled request: #{e.message}"
+ end
+
+ def name_for(env)
+ slug = env["PATH_INFO"].to_s.gsub(%r{[^\w]+}, "-").delete_prefix("-").delete_suffix("-")
+ slug = "root" if slug.empty?
+ "#{env['REQUEST_METHOD'].to_s.downcase}-#{slug}-#{digest(env)}"
+ end
+
+ def digest(env)
+ Digest::SHA256.hexdigest([env["REQUEST_METHOD"], env["PATH_INFO"], request_body(env)].join("\n"))[0, 8]
+ end
+
+ def render_request(env)
+ lines = ["#{env['REQUEST_METHOD']} #{full_path(env)} #{env.fetch('SERVER_PROTOCOL', 'HTTP/1.1')}"]
+ lines += each_header(env).map { |name, value| "#{name}: #{value}" }
+ message(lines, request_body(env))
+ end
+
+ def render_response(status, headers, body)
+ message([status.to_s] + headers.map { |name, value| "#{name}: #{value}" }, body)
+ end
+
+ def message(lines, body)
+ head = lines.join("\n") + "\n\n"
+ body = body.to_s.chomp
+ body.empty? ? head : "#{head}#{body}\n"
+ end
+
+ def full_path(env)
+ path = env["PATH_INFO"].to_s
+ query = env["QUERY_STRING"].to_s
+ query.empty? ? path : "#{path}?#{query}"
+ end
+
+ def each_header(env)
+ env.filter_map { |key, value|
+ case key
+ when /\AHTTP_(.+)\z/ then [header_name(Regexp.last_match(1)), value]
+ when "CONTENT_TYPE" then ["Content-Type", value]
+ when "CONTENT_LENGTH" then ["Content-Length", value]
+ end
+ }.sort
+ end
+
+ def header_name(name)
+ name.split("_").map(&:capitalize).join("-")
+ end
+
+ def request_body(env)
+ input = env["rack.input"]
+ return "" if input.nil?
+
+ body = input.read.to_s
+ input.rewind
+ body
+ end
+ end
+end
diff --git a/lib/pro_tacts/web.rb b/lib/pro_tacts/web.rb
index ef72f21..73604c8 100644
--- a/lib/pro_tacts/web.rb
+++ b/lib/pro_tacts/web.rb
@@ -9,6 +9,7 @@ require "roda"
require "pro_tacts/debug_logger"
require "pro_tacts/addressbook"
require "pro_tacts/tailscale_auth"
+require "pro_tacts/unhandled_requests"
require "roda/plugins/dav_verbs"
module ProTacts
@@ -22,6 +23,10 @@ module ProTacts
# not get its body dumped to the log.
use ProTacts::TailscaleAuth
+ # Below the auth gate: a refused request is not missing functionality,
+ # and recording one would write an unauthenticated body to disk.
+ use ProTacts::UnhandledRequests, directory: ProTacts.config.unhandled_dir
+
if ProTacts.config.debug?
logger = ProTacts::DebugLogger.open_log(ProTacts.config.debug_log_path)
use ProTacts::DebugLogger, logger: logger
diff --git a/test/pro_tacts/test_unhandled_requests.rb b/test/pro_tacts/test_unhandled_requests.rb
new file mode 100644
index 0000000..4e3ea54
--- /dev/null
+++ b/test/pro_tacts/test_unhandled_requests.rb
@@ -0,0 +1,144 @@
+require_relative "../test_helper"
+require "rack/test"
+require "tmpdir"
+
+require "pro_tacts/unhandled_requests"
+
+class UnhandledRequestsTest < Minitest::Test
+ include Rack::Test::Methods
+
+ # Returns whatever status the test asks for, so each case can drive the
+ # capture decision directly.
+ class Stub
+ attr_accessor :status
+
+ def initialize
+ @status = 200
+ end
+
+ def call(_env)
+ [@status, { "Content-Type" => "text/plain" }, ["body from the app"]]
+ end
+ end
+
+ def setup
+ @directory = Pathname.new(Dir.mktmpdir("unhandled"))
+ @stub = Stub.new
+ @app = ProTacts::UnhandledRequests.new(@stub, directory: @directory)
+ end
+
+ def teardown
+ FileUtils.remove_entry(@directory)
+ end
+
+ attr_reader :app
+
+ def captures
+ @directory.children.select(&:directory?)
+ end
+
+ def test_successful_requests_are_not_captured
+ @stub.status = 200
+
+ get "/dav/addressbook/"
+
+ assert_empty captures
+ end
+
+ def test_a_404_is_captured
+ @stub.status = 404
+
+ get "/dav/unknown/"
+
+ assert_equal 1, captures.size
+ end
+
+ def test_a_server_error_is_captured
+ @stub.status = 500
+
+ get "/dav/addressbook/"
+
+ assert_equal 1, captures.size
+ end
+
+ def test_capture_records_the_request_verbatim
+ @stub.status = 404
+
+ request "/dav/addressbook/", method: "REPORT", input: "<sync-collection/>",
+ "CONTENT_TYPE" => "text/xml", "HTTP_DEPTH" => "1"
+
+ recorded = (captures.first / "request").read
+
+ assert_match(%r{\AREPORT /dav/addressbook/ HTTP/1\.\d\n}, recorded)
+ assert_includes recorded, "Depth: 1"
+ assert_includes recorded, "Content-Type: text/xml"
+ assert_includes recorded, "<sync-collection/>"
+ end
+
+ def test_capture_records_the_response
+ @stub.status = 404
+
+ get "/dav/unknown/"
+
+ recorded = (captures.first / "response").read
+
+ assert_includes recorded, "404"
+ assert_includes recorded, "Content-Type: text/plain"
+ assert_includes recorded, "body from the app"
+ end
+
+ # The whole point of the format: a capture drops into the fixture suite.
+ def test_capture_uses_the_fixture_layout
+ @stub.status = 404
+
+ get "/dav/unknown/"
+
+ assert_equal %w[request response], captures.first.children.map { it.basename.to_s }.sort
+ end
+
+ def test_the_app_still_sees_its_own_body
+ @stub.status = 404
+
+ get "/dav/unknown/"
+
+ assert_equal "body from the app", last_response.body
+ end
+
+ def test_a_repeated_request_is_captured_once
+ @stub.status = 404
+
+ 3.times { get "/dav/unknown/" }
+
+ assert_equal 1, captures.size
+ end
+
+ def test_different_requests_are_captured_separately
+ @stub.status = 404
+
+ get "/dav/unknown/"
+ get "/dav/other/"
+
+ assert_equal 2, captures.size
+ end
+
+ # Same path, different body: the multiget case, where what was asked for
+ # is the part that matters.
+ def test_the_body_distinguishes_captures
+ @stub.status = 404
+
+ request "/dav/addressbook/", method: "REPORT", input: "<one/>"
+ request "/dav/addressbook/", method: "REPORT", input: "<two/>"
+
+ assert_equal 2, captures.size
+ end
+
+ def test_an_unwritable_directory_does_not_break_the_response
+ app = ProTacts::UnhandledRequests.new(@stub, directory: @directory / "nested" / "deep")
+ @stub.status = 404
+
+ session = Rack::Test::Session.new(Rack::MockSession.new(app))
+ session.get "/dav/unknown/"
+
+ assert_equal 404, session.last_response.status
+ end
+end
diff --git a/test/pro_tacts/test_web.rb b/test/pro_tacts/test_web.rb
index 9a9afbe..92b8a04 100644
--- a/test/pro_tacts/test_web.rb
+++ b/test/pro_tacts/test_web.rb
@@ -1,6 +1,7 @@
require_relative "../test_helper"
require "digest"
+require "fileutils"
require "pathname"
require "rack/test"
require "tmpdir"
@@ -107,6 +108,37 @@ class WebTest < Minitest::Test
assert_includes last_response.body, "AB12C345-6789-0DEF-1234-567890ABCDEF.vcf"
end
+ # Guards the wiring rather than the middleware: mounted in the stack, below
+ # the auth gate, pointed at the configured directory.
+ def test_an_unhandled_request_is_kept_on_disk
+ directory = ProTacts.config.unhandled_dir
+ FileUtils.rm_rf(directory)
+
+ get "/dav/addressbook/no-such-contact.vcf"
+
+ assert_equal 404, last_response.status
+
+ captured = Pathname.new(directory).glob("*/request").map(&:read)
+
+ assert_equal 1, captured.size
+ assert_includes captured.first, "/dav/addressbook/no-such-contact.vcf"
+ ensure
+ FileUtils.rm_rf(directory)
+ end
+
+ def test_a_refused_request_is_not_kept_on_disk
+ directory = ProTacts.config.unhandled_dir
+ FileUtils.rm_rf(directory)
+
+ header "Tailscale-User-Login", ""
+ get "/dav/addressbook/no-such-contact.vcf"
+
+ assert_equal 403, last_response.status
+ refute Pathname.new(directory).exist?, "a refused request should leave nothing behind"
+ ensure
+ FileUtils.rm_rf(directory)
+ end
+
def test_get_unknown_contact_is_404
get "/dav/addressbook/nope.vcf"
diff --git a/test/test_helper.rb b/test/test_helper.rb
index eca876f..a30fcd8 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -7,4 +7,8 @@ require "pathname"
ENV["PRO_TACTS_DATA_DIR"] = (Pathname.new(__dir__) / "fixtures").to_s
+# Several tests provoke 404s. Keep the captures out of log/ and out of the
+# fixtures; UnhandledRequestsTest points the middleware at its own tmpdir.
+ENV["PRO_TACTS_UNHANDLED_DIR"] = (Pathname.new(__dir__).parent / "tmp" / "test-unhandled").to_s
+
require "minitest/autorun"