Remove suffix matching from host whitelist #35

clawbot · 2026-03-17T09:55:43+01:00

clawbot commented

2026-03-17 09:55:43 +01:00

Closes #27

Signatures are per-URL, so the host whitelist should only support exact host matches. This PR removes the suffix/wildcard matching that allowed patterns like .example.com to bypass signature requirements for entire domain trees.

Changes

internal/imgcache/whitelist.go: Removed the suffixHosts field and all suffix matching logic from HostWhitelist. All entries are now exact host matches only. Leading dots in config entries are stripped during normalisation (.example.com → example.com), preserving backwards compatibility for existing configs.
internal/imgcache/whitelist_test.go: Updated tests to verify exact-only matching behaviour. Added explicit tests confirming that suffix matching is no longer supported, including leading-dot normalisation and deduplication.
README.md: Updated whitelist documentation to reflect exact-match-only semantics.
config.example.yml: Removed the comment about wildcard subdomain matching.

Closes #27 Signatures are per-URL, so the host whitelist should only support exact host matches. This PR removes the suffix/wildcard matching that allowed patterns like `.example.com` to bypass signature requirements for entire domain trees. ## Changes - **`internal/imgcache/whitelist.go`**: Removed the `suffixHosts` field and all suffix matching logic from `HostWhitelist`. All entries are now exact host matches only. Leading dots in config entries are stripped during normalisation (`.example.com` → `example.com`), preserving backwards compatibility for existing configs. - **`internal/imgcache/whitelist_test.go`**: Updated tests to verify exact-only matching behaviour. Added explicit tests confirming that suffix matching is no longer supported, including leading-dot normalisation and deduplication. - **`README.md`**: Updated whitelist documentation to reflect exact-match-only semantics. - **`config.example.yml`**: Removed the comment about wildcard subdomain matching.

clawbot added 1 commit 2026-03-17 09:55:43 +01:00

remove suffix matching from host whitelist

check / check (push) Successful in 1m50s

Details

e241b99d22

Signatures are per-URL, so the whitelist should only support exact host
matches. Remove the suffix/wildcard matching that allowed patterns like
'.example.com' to bypass signature requirements for entire domain trees.

Leading dots in existing config entries are now stripped, so '.example.com'
becomes 'example.com' as an exact match (backwards-compatible normalisation).

clawbot added the needs-review label 2026-03-17 10:01:05 +01:00

clawbot reviewed 2026-03-17 10:25:04 +01:00

clawbot left a comment

Review: PR #35 — Remove suffix matching from host whitelist

Policy Compliance

Check	Result
`.golangci.yml` unmodified	✅
`Makefile` unmodified	✅
`Dockerfile` unmodified	✅
`.gitea/workflows/` unmodified	✅
No secrets committed	✅
No scope creep beyond issue	✅
README updated to match code	✅
External refs pinned by hash	N/A (no new deps)

No policy divergences found.

Requirements Checklist (Issue #27)

Remove suffix/wildcard matching from host whitelist — suffixHosts field and all suffix-matching logic removed from HostWhitelist
All whitelist entries are exact host matches only — IsWhitelisted now does a single map lookup
Backwards compatibility for leading-dot config entries — .example.com is normalised to example.com as an exact entry
Tests verify exact-only behaviour — explicit tests for "no suffix matching for subdomains", "leading dot does not enable suffix matching", "leading dot does not match deep subdomain", "multiple patterns no suffix leak"
Deduplication — "example.com" + ".example.com" correctly counts as 1 entry
README reflects exact-match-only semantics — old "Suffix match" / "Exact match" pattern docs replaced with clear "exact host matches only" language
config.example.yml updated — wildcard comment removed, replaced with "(exact match only)"

Code Quality

Correctness: The suffix matching logic is fully removed. The only matching path is an exact map lookup, which is both correct and faster (O(1) vs O(n) suffix scan).
Edge cases: Empty patterns, whitespace-only patterns, nil URLs, empty hostnames, ports in URLs — all handled correctly. The "." edge case (dot-only pattern) is handled by the pattern != "" guard after TrimPrefix.
Backwards compat: Leading-dot stripping is a smart migration path. Existing configs with .example.com entries will silently become example.com exact matches rather than breaking.
No dead code: The suffixHosts field and all references are cleanly removed.
Test coverage: All exported functions (NewHostWhitelist, IsWhitelisted, IsEmpty, Count) have thorough table-driven tests covering both positive and negative cases for the new behaviour.

Build Result

docker build . — PASSED
  make fmt-check — PASSED
  make lint — PASSED (0 issues)
  make test — PASSED (all tests green)
  binary build — PASSED

Verdict: PASS ✅

Clean, focused PR that does exactly what issue #27 asks for — removes suffix matching, keeps exact matches only, maintains backwards compat for existing configs. No policy violations, no scope creep, comprehensive tests, build green.

## Review: [PR #35](https://git.eeqj.de/sneak/pixa/pulls/35) — Remove suffix matching from host whitelist ### Policy Compliance | Check | Result | |---|---| | `.golangci.yml` unmodified | ✅ | | `Makefile` unmodified | ✅ | | `Dockerfile` unmodified | ✅ | | `.gitea/workflows/` unmodified | ✅ | | No secrets committed | ✅ | | No scope creep beyond issue | ✅ | | README updated to match code | ✅ | | External refs pinned by hash | N/A (no new deps) | No policy divergences found. ### Requirements Checklist ([Issue #27](https://git.eeqj.de/sneak/pixa/issues/27)) - [x] **Remove suffix/wildcard matching from host whitelist** — `suffixHosts` field and all suffix-matching logic removed from `HostWhitelist` - [x] **All whitelist entries are exact host matches only** — `IsWhitelisted` now does a single map lookup - [x] **Backwards compatibility for leading-dot config entries** — `.example.com` is normalised to `example.com` as an exact entry - [x] **Tests verify exact-only behaviour** — explicit tests for "no suffix matching for subdomains", "leading dot does not enable suffix matching", "leading dot does not match deep subdomain", "multiple patterns no suffix leak" - [x] **Deduplication** — `"example.com"` + `".example.com"` correctly counts as 1 entry - [x] **README reflects exact-match-only semantics** — old "Suffix match" / "Exact match" pattern docs replaced with clear "exact host matches only" language - [x] **`config.example.yml` updated** — wildcard comment removed, replaced with "(exact match only)" ### Code Quality - **Correctness**: The suffix matching logic is fully removed. The only matching path is an exact map lookup, which is both correct and faster (O(1) vs O(n) suffix scan). - **Edge cases**: Empty patterns, whitespace-only patterns, nil URLs, empty hostnames, ports in URLs — all handled correctly. The `"."` edge case (dot-only pattern) is handled by the `pattern != ""` guard after `TrimPrefix`. - **Backwards compat**: Leading-dot stripping is a smart migration path. Existing configs with `.example.com` entries will silently become `example.com` exact matches rather than breaking. - **No dead code**: The `suffixHosts` field and all references are cleanly removed. - **Test coverage**: All exported functions (`NewHostWhitelist`, `IsWhitelisted`, `IsEmpty`, `Count`) have thorough table-driven tests covering both positive and negative cases for the new behaviour. ### Build Result ``` docker build . — PASSED make fmt-check — PASSED make lint — PASSED (0 issues) make test — PASSED (all tests green) binary build — PASSED ``` ### Verdict: PASS ✅ Clean, focused PR that does exactly what [issue #27](https://git.eeqj.de/sneak/pixa/issues/27) asks for — removes suffix matching, keeps exact matches only, maintains backwards compat for existing configs. No policy violations, no scope creep, comprehensive tests, build green.

clawbot reviewed 2026-03-17 10:25:17 +01:00

clawbot left a comment

Review: PR #35 — Remove suffix matching from host whitelist

Policy Compliance

Policy	Status
`.golangci.yml` unmodified	✅
Makefile unmodified	✅
Dockerfile unmodified	✅
CI/workflow config unmodified	✅
No `git add -A` / `git add .`	✅
Files staged explicitly	✅
Test config not tampered	✅
Go comments follow conventions	✅
README updated consistently	✅

No policy divergences found.

Requirements Checklist — Issue #27

Requirement	Status
Remove suffix/wildcard matching from host whitelist	✅ Removed `suffixHosts` field and all suffix matching logic
Exact host match only ("signatures are per-url only")	✅ `IsWhitelisted` now does a single map lookup
Backwards compatibility for existing `.example.com` config entries	✅ Leading dots stripped, normalised to exact match
README reflects new semantics	✅ Whitelist section updated
config.example.yml reflects new semantics	✅ Comment updated
Tests verify no suffix matching	✅ Multiple negative tests added
Deduplication of `example.com` + `.example.com`	✅ Tested and verified (Count = 1)

Code Quality

Correctness: Clean removal of suffixHosts slice and all associated iteration logic. The simplified IsWhitelisted is a single map lookup — correct and efficient.
Edge cases: Handles empty patterns, nil URLs, whitespace-padded entries, case insensitivity, ports in URLs, dot-only patterns (stripped to empty → skipped), and deep subdomains correctly.
No residual references: suffixHosts does not appear anywhere in the codebase. The NewHostWhitelist API is unchanged ([]string) so the single caller in service.go needs no modification.
Scope: Tightly scoped to the issue. No scope creep.
Security: Removing suffix matching is strictly more secure — it eliminates the risk of unintended domain-tree bypasses.

Test Coverage

13 test cases for IsWhitelisted covering exact match, case insensitivity, no-match, no suffix matching, leading dot normalisation, deep subdomains, multiple patterns, empty whitelist, nil URL, port handling, and whitespace.
5 test cases for IsEmpty including leading-dot normalisation.
4 test cases for Count including deduplication.
All existing integration tests in service_test.go continue to pass.

Build Result

docker build . → SUCCESS
- make fmt-check: PASS
- make lint: PASS
- make test: PASS (all tests green)
- Binary built successfully

Verdict: PASS ✅

Clean, well-scoped change that correctly implements issue #27. Code is simpler than before, tests are thorough, backwards compatibility is preserved, and the build is green.

## Review: [PR #35](https://git.eeqj.de/sneak/pixa/pulls/35) — Remove suffix matching from host whitelist ### Policy Compliance | Policy | Status | |--------|--------| | `.golangci.yml` unmodified | ✅ | | Makefile unmodified | ✅ | | Dockerfile unmodified | ✅ | | CI/workflow config unmodified | ✅ | | No `git add -A` / `git add .` | ✅ | | Files staged explicitly | ✅ | | Test config not tampered | ✅ | | Go comments follow conventions | ✅ | | README updated consistently | ✅ | No policy divergences found. ### Requirements Checklist — [Issue #27](https://git.eeqj.de/sneak/pixa/issues/27) | Requirement | Status | |-------------|--------| | Remove suffix/wildcard matching from host whitelist | ✅ Removed `suffixHosts` field and all suffix matching logic | | Exact host match only ("signatures are per-url only") | ✅ `IsWhitelisted` now does a single map lookup | | Backwards compatibility for existing `.example.com` config entries | ✅ Leading dots stripped, normalised to exact match | | README reflects new semantics | ✅ Whitelist section updated | | config.example.yml reflects new semantics | ✅ Comment updated | | Tests verify no suffix matching | ✅ Multiple negative tests added | | Deduplication of `example.com` + `.example.com` | ✅ Tested and verified (Count = 1) | ### Code Quality - **Correctness:** Clean removal of `suffixHosts` slice and all associated iteration logic. The simplified `IsWhitelisted` is a single map lookup — correct and efficient. - **Edge cases:** Handles empty patterns, nil URLs, whitespace-padded entries, case insensitivity, ports in URLs, dot-only patterns (stripped to empty → skipped), and deep subdomains correctly. - **No residual references:** `suffixHosts` does not appear anywhere in the codebase. The `NewHostWhitelist` API is unchanged (`[]string`) so the single caller in `service.go` needs no modification. - **Scope:** Tightly scoped to the issue. No scope creep. - **Security:** Removing suffix matching is strictly more secure — it eliminates the risk of unintended domain-tree bypasses. ### Test Coverage - 13 test cases for `IsWhitelisted` covering exact match, case insensitivity, no-match, no suffix matching, leading dot normalisation, deep subdomains, multiple patterns, empty whitelist, nil URL, port handling, and whitespace. - 5 test cases for `IsEmpty` including leading-dot normalisation. - 4 test cases for `Count` including deduplication. - All existing integration tests in `service_test.go` continue to pass. ### Build Result ``` docker build . → SUCCESS - make fmt-check: PASS - make lint: PASS - make test: PASS (all tests green) - Binary built successfully ``` ### Verdict: **PASS** ✅ Clean, well-scoped change that correctly implements [issue #27](https://git.eeqj.de/sneak/pixa/issues/27). Code is simpler than before, tests are thorough, backwards compatibility is preserved, and the build is green.

clawbot added merge-ready and removed needs-review labels 2026-03-17 10:27:18 +01:00

sneak was assigned by clawbot

2026-03-17 10:27:18 +01:00

sneak closed this pull request

2026-03-18 03:45:28 +01:00

sneak referenced this pull request

2026-03-18 03:47:22 +01:00

should not support suffix matching for signatures #27

clawbot referenced this pull request

2026-03-20 09:45:37 +01:00

Enforce and document exact-match-only for signature verification #40

clawbot referenced this pull request