Add opt-in access control for infrastructure operators by DanGould · Pull Request #1330 · payjoin/rust-payjoin

DanGould · 2026-02-15T12:08:16Z

This came straight from the trough but I don't think it's actually slop.

I had opus 4.6 draft this (much faster turnover, better integration with GitHub apparently) and codex 5.3 review it and then did a bunch of manual review with the help of codex to get CI passing and understand the design tradeoffs.. nix flaking took forever (1h+), maybe because I just reinstalled nix

Summary

Adds a feature-gated (access-control) opt-in system for infrastructure operators to control access to their payjoin-mailroom instances. All behavior is off by default and requires explicit configuration.

GeoIP region filtering — middleware blocks connections from configurable ISO 3166-1 alpha-2 regions. Auto-fetches DB-IP Lite (CC-BY 4.0) if no custom MMDB path is provided.
V1 address screening — parses V1 PSBTs and checks output/input addresses against a configurable blocklist file.
Address list auto-fetch — background task fetches blocklist from a configurable URL on a refresh interval, with disk cache fallback.
V1 enable toggle — config flag to accept all V1 requests otherwise rejected.
V1 data minimization — clears plaintext V1 payload from memory immediately after first read.

Test plan

21 tests passing across payjoin-mailroom and payjoin-directory
Compiles cleanly with and without access-control feature flag
cargo clippy --all-features clean
Manual testing with a real GeoIP database and blocklist
Review test MMDB fixture (test-data/GeoIP2-Country-Test.mmdb) is appropriate for CI

coveralls · 2026-02-15T12:12:56Z

Pull Request Test Coverage Report for Build 22094646540

Details

331 of 558 (59.32%) changed or added relevant lines in 9 files are covered.
3 unchanged lines in 3 files lost coverage.
Overall coverage decreased (-0.9%) to 82.25%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin-directory/src/db/mod.rs	0	1	0.0%
payjoin-directory/src/main.rs	0	1	0.0%
payjoin-mailroom/src/config.rs	4	5	80.0%
payjoin-test-utils/src/lib.rs	16	18	88.89%
payjoin-directory/src/db/files.rs	41	45	91.11%
payjoin-mailroom/src/middleware.rs	17	25	68.0%
payjoin-directory/src/lib.rs	104	135	77.04%
payjoin-mailroom/src/access_control.rs	79	167	47.31%
payjoin-mailroom/src/lib.rs	70	161	43.48%

Files with Coverage Reduction	New Missed Lines	%
payjoin-directory/src/db/files.rs	1	91.74%
payjoin-directory/src/db/mod.rs	1	0.0%
payjoin-mailroom/src/config.rs	1	44.07%

Totals
Change from base Build 22022393890:	-0.9%
Covered Lines:	10542
Relevant Lines:	12817

💛 - Coveralls

Introduce an opt-in `access-control` feature flag that enables IP-based geographic filtering at the axum middleware layer. When configured with blocked region codes (ISO 3166-1 alpha-2 [1]), incoming connections from those regions are rejected before any protocol-level processing. The system auto-fetches a free DB-IP Lite MMDB database when `blocked_regions` is configured but no custom `geo_db_path` is provided. Lookups fail-open on errors to avoid false positives. A small in-repo MMDB fixture is used in tests to keep GeoIP checks offline and deterministic while still exercising real MMDB parsing and lookup paths. [1]: https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2

Parse V1 PSBT payloads to extract bitcoin addresses from inputs and outputs, checking each against a locally loaded blocklist file. Requests containing blocked addresses receive a 403 response before the payload is stored. Unparseable PSBTs fail-open since they cannot complete a transaction anyway. The blocklist is loaded from a configurable file path at startup and shared via Arc<RwLock<HashSet>> to support later dynamic updates.

Support fetching the blocked address list from a configurable URL with periodic refresh (default 24h). On startup, attempt an initial fetch; on failure, fall back to a cached copy on disk. A background task keeps the shared blocklist updated without restart.

V1 payloads are plaintext PSBTs held in memory. Previously the payload persisted until the sender timed out or the receiver responded. Now the payload is take()n on first read — subsequent reads return None while the oneshot response channel remains alive for the receiver's reply. This minimizes the window during which plaintext transaction data resides in process memory.

spacebear21

This came straight from the trough but I don't think it's actually slop.

It's definitely way sloppier than your usual code quality 🥲 I reviewed the first four commits and found a lot to be desired, posting what I have so far

spacebear21 · 2026-02-17T16:53:28Z

+
+pub struct AccessControl {
+    geo_reader: Option<maxminddb::Reader<Vec<u8>>>,
+    blocked_regions: HashSet<String>,


It seems natural that it should also have a custom blocked_ips HashSet for custom IP ranges.

Agree. Nothing stopping us from shipping that. This is an absolute bare minimum off the shelf strategy to work with easy to access DBs

spacebear21 · 2026-02-17T16:58:01Z

+        let geo_reader = match &config.geo_db_path {
+            Some(path) => Some(maxminddb::Reader::open_readfile(path)?),
+            None if !config.blocked_regions.is_empty() => {
+                let cached = storage_dir.join("geoip.mmdb");


nit: if other access control things will be stored (e.g. address blocklists), we should group them all in a access control subdir

spacebear21 · 2026-02-17T17:00:55Z

+fn year_month_from_days_since_epoch(days_since_epoch: i64) -> (i32, u32) {
+    // Exact conversion from Unix days to Gregorian year/month in UTC.
+    // Based on Howard Hinnant's civil calendar algorithm.
+    let z = days_since_epoch + 719_468;
+    let era = if z >= 0 { z } else { z - 146_096 } / 146_097;
+    let doe = z - era * 146_097;
+    let yoe = (doe - doe / 1_460 + doe / 36_524 - doe / 146_096) / 365;
+    let y = yoe + era * 400;
+    let doy = doe - (365 * yoe + yoe / 4 - yoe / 100);
+    let mp = (5 * doy + 2) / 153;
+    let month = (mp + if mp < 10 { 3 } else { -9 }) as u32;
+    let year = (y + if month <= 2 { 1 } else { 0 }) as i32;
+    (year, month)
+}


This looks like LLM insanity, does std::time not have a utility that could achieve the same effect? Looks like we just need the current year and month?

std::time does not have a utility to achieve the same affect, we'd need to introduce chrono or time.

Yes, this was plainly and obviously LLM written but also checked thereafter and recognized that it's used to fetch the updated DB on a schedule. Kind of like how wikipedia is really good for technical / algorithmic explanation, so is LLM for concrete well understood algorithms and fetch unit tests for well-known time edge cases. This algo is used to fetch the date to get an updated DB at the right time, I think it's acceptable to take this rather than a dependency for that purpose.

spacebear21 · 2026-02-17T17:05:21Z

              "payjoin-cli" = "--features v1,v2";
              "payjoin-directory" = "";
              "ohttp-relay" = "";
              "payjoin-mailroom" = "--features acme,telemetry";


We need to enable the access-control feature here if we want it to be accessible in the nix flake and docker images built with nix2container.

spacebear21 · 2026-02-17T17:09:07Z

+                .status(StatusCode::FORBIDDEN)
+                .header(CONTENT_TYPE, "application/json")
+                .body(full(
+                    r#"{"errorCode": "v1-disabled", "message": "V1 is disabled on this server"}"#,


Should this use the version-unsupported BIP78 error code? Per the BIP "it is important that error codes that are not well-known and that the message do not appear on the sender's software user interface"

Yes definitely, if not the UI will just show a generic error message

spacebear21 · 2026-02-17T17:26:32Z

+pub fn load_blocked_addresses(path: &Path) -> anyhow::Result<HashSet<String>> {
+    let content = std::fs::read_to_string(path)?;
+    Ok(content
+        .lines()
+        .map(|l| normalize_blocked_address(l.trim()))
+        .filter(|l| !l.is_empty())
+        .collect())
+}
+
+pub fn normalize_blocked_address(addr: &str) -> String {
+    if is_bech32_address(addr) {
+        addr.to_ascii_lowercase()
+    } else {
+        addr.to_string()
+    }
+}
+
+fn is_bech32_address(addr: &str) -> bool {
+    let lower = addr.to_ascii_lowercase();
+    lower.starts_with("bc1") || lower.starts_with("tb1") || lower.starts_with("bcrt1")
+}
+


Why not parse these as bitcoin::Address here on read, and let rust-bitcoin handle weird bitcoin things on parsing and equality checks?

Because it's at the mailroom rather than the directory and we're not parsing addresses we're just matching the strings. I can instead have the mailroom do the fetching and just have the directory load the addresses from a path and do loading and normalizing, but I do think this would have worked as-is without giving us a ton of trouble.

spacebear21 · 2026-02-17T17:28:34Z

    }
 }

+pub fn load_blocked_addresses(path: &Path) -> anyhow::Result<HashSet<String>> {


Shouldn't AccessControl be in charge of loading and checking blocked addresses? Are they only separate to accommodate payjoin-directory still living in a separate crate?

No, AccessControl is the Region-specific control, relevant to the top-level mailroom. Blocked addresses indeed are a separate struct / update path to be passed specifically to payjoin-directory as a separate module & lower level of abstraction, whether it's in a separate crate or not doesn't matter here.

It could be made to encapsulate both geo and blocked addresses but right now they're separate

spacebear21 · 2026-02-17T17:34:31Z

+                    ) {
+                        tracing::warn!("Failed to write address cache: {e}");
+                    }
+                    info!("Fetched {} blocked addresses from URL", fetched.len());
+                    *blocked.write().await = fetched;
+                }
+                Err(e) => {
+                    tracing::warn!("Failed to read address list response: {e}");
+                    try_load_cache(&cache_path, &blocked).await;
+                }
+            },
+            Ok(resp) => {
+                tracing::warn!("Failed to fetch address list: HTTP {}", resp.status());
+                try_load_cache(&cache_path, &blocked).await;
+            }
+            Err(e) => {
+                tracing::warn!("Failed to fetch address list: {e}");
+                try_load_cache(&cache_path, &blocked).await;
+            }
+        }


error handling/tracing here seems excessive

It'd help me correct this if you'd define why you think this is excessive, and what level of granularity you'd accept. These are errors on once every 24h checks, and if they fail it'd be good to know precisely why

spacebear21 · 2026-02-17T17:40:35Z

    }
 }

+fn get_v1_disabled(config: &Config) -> bool {


Why even have this, it's just flipping the enable_v1 config value. init_directory already takes a config, no need to pass a separate v1_disabled parameter

spacebear21 · 2026-02-17T17:44:25Z

    pub blocked_addresses_path: Option<PathBuf>,
    pub blocked_addresses_url: Option<String>,
    pub blocked_addresses_refresh_secs: Option<u64>,
+    pub enable_v1: bool,


I'm not sure this should be scoped to AccessControlConfig, it's not strictly access control, an operator might simply not want to store plaintext. Also it's confusing because currently v1 is enabled by default, unless --features=access-control in which case it's disabled by default and needs to be re-enabled explicitly.

I agree this config isn't quite right

DanGould · 2026-02-18T03:28:26Z

It's definitely way sloppier than your usual code quality 🥲 I reviewed the first four commits and found a lot to be desired, posting what I have so far

I agree, and there's a conversation to be had about the acceptable level of sloppiness for something that's 1. on the periphery of our integrations rather than library core 2. a burning time crunch blocking us from proceeding elsewhere.

Your comments about

the error message for v1
Features necessary for flake builds

do need to be addressed here asap to ship. The other comment about

the way the configuration of v1 ability could be better

is noted.

Can you be specific about what's left to be desired? This kind of change seems largely mechanical and peripheral to our main integration goals, which is why I felt a whole week of my time focused on this was not worth it compared to shipping something that works and isn't broken that some folks with less experience and more free cycles can iterate on and refine (as they already are doing)

DanGould · 2026-02-19T14:10:09Z

Split into #1336 / #1337

DanGould force-pushed the access-control branch 7 times, most recently from d67d662 to e00fd05 Compare February 16, 2026 14:31

DanGould requested a review from spacebear21 February 16, 2026 15:11

DanGould marked this pull request as ready for review February 16, 2026 15:11

DanGould marked this pull request as draft February 16, 2026 15:26

DanGould removed the request for review from spacebear21 February 16, 2026 15:27

DanGould force-pushed the access-control branch 3 times, most recently from 1585aa4 to c95cece Compare February 17, 2026 10:15

DanGould added 7 commits February 17, 2026 18:20

Add config toggle to enable V1 explicitly

e456e84

Add mailroom config example with access control section

1513060

Expand example blocked regions list in mailroom config

c1326da

DanGould force-pushed the access-control branch from c95cece to c1326da Compare February 17, 2026 10:21

DanGould marked this pull request as ready for review February 17, 2026 14:31

DanGould requested a review from spacebear21 February 17, 2026 14:32

spacebear21 requested changes Feb 17, 2026

View reviewed changes

DanGould closed this Feb 19, 2026

DanGould mentioned this pull request Feb 23, 2026

Standup Input: Week of 2026-02-23 #1352

Closed

Conversation

DanGould commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Uh oh!

coveralls commented Feb 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 22094646540

Details

💛 - Coveralls

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DanGould commented Feb 18, 2026

Uh oh!

DanGould commented Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

DanGould commented Feb 15, 2026 •

edited

Loading

coveralls commented Feb 15, 2026 •

edited

Loading