sneak/pixa
1
0
Fork 0
Code Issues 3 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

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

Merged
sneak merged 1 commits from fix/signature-exact-match-only into main 2026-03-20 23:56:45 +01:00
Conversation 1 Commits 1 Files Changed 4 +251 -1
clawbot commented 2026-03-20 09:45:37 +01:00
Collaborator
Copy Link

Closes #27

Signatures are per-URL only — this PR adds explicit tests and documentation enforcing that HMAC-SHA256 signatures verify against exact URLs only. No suffix matching, wildcard matching, or partial matching is supported.

What this does NOT touch

The host whitelist code (whitelist.go) is not modified. This PR is exclusively about signature verification, per sneak's instructions on issue #27, PR #32, and PR #35.

Changes

internal/imgcache/signature.go

  • Added documentation comments on Verify() and buildSignatureData() explicitly specifying that signatures are exact-match only — no suffix, wildcard, or partial matching

internal/imgcache/signature_test.go

  • TestSigner_Verify_ExactMatchOnly: 14 tamper cases verifying that modifying any signed component (host, path, query, dimensions, format) causes verification to fail. Host-specific cases include:
    • Parent domain (example.com) does not match subdomain signature (cdn.example.com)
    • Sibling subdomain (images.example.com) does not match
    • Deeper subdomain (images.cdn.example.com) does not match
    • Evil suffix domain (cdn.example.com.evil.com) does not match
    • Prefixed host (evilcdn.example.com) does not match
  • TestSigner_Sign_ExactHostInData: Verifies that suffix-related hosts (cdn.example.com, example.com, images.example.com, etc.) all produce distinct signatures

internal/imgcache/service_test.go

  • TestService_ValidateRequest_SignatureExactHostMatch: Integration test through ValidateRequest verifying that a valid signature for cdn.example.com is rejected when presented with a different host (parent domain, sibling subdomain, deeper subdomain, evil suffix, prefixed host)

README.md

  • Updated Signature Specification section to explicitly document exact-match-only semantics
Closes https://git.eeqj.de/sneak/pixa/issues/27 Signatures are per-URL only — this PR adds explicit tests and documentation enforcing that HMAC-SHA256 signatures verify against exact URLs only. No suffix matching, wildcard matching, or partial matching is supported. ## What this does NOT touch **The host whitelist code (`whitelist.go`) is not modified.** This PR is exclusively about signature verification, per sneak's instructions on [issue #27](https://git.eeqj.de/sneak/pixa/issues/27), [PR #32](https://git.eeqj.de/sneak/pixa/pulls/32), and [PR #35](https://git.eeqj.de/sneak/pixa/pulls/35). ## Changes ### `internal/imgcache/signature.go` - Added documentation comments on `Verify()` and `buildSignatureData()` explicitly specifying that signatures are exact-match only — no suffix, wildcard, or partial matching ### `internal/imgcache/signature_test.go` - **`TestSigner_Verify_ExactMatchOnly`**: 14 tamper cases verifying that modifying any signed component (host, path, query, dimensions, format) causes verification to fail. Host-specific cases include: - Parent domain (`example.com`) does not match subdomain signature (`cdn.example.com`) - Sibling subdomain (`images.example.com`) does not match - Deeper subdomain (`images.cdn.example.com`) does not match - Evil suffix domain (`cdn.example.com.evil.com`) does not match - Prefixed host (`evilcdn.example.com`) does not match - **`TestSigner_Sign_ExactHostInData`**: Verifies that suffix-related hosts (`cdn.example.com`, `example.com`, `images.example.com`, etc.) all produce distinct signatures ### `internal/imgcache/service_test.go` - **`TestService_ValidateRequest_SignatureExactHostMatch`**: Integration test through `ValidateRequest` verifying that a valid signature for `cdn.example.com` is rejected when presented with a different host (parent domain, sibling subdomain, deeper subdomain, evil suffix, prefixed host) ### `README.md` - Updated Signature Specification section to explicitly document exact-match-only semantics
clawbot added 1 commit 2026-03-20 09:45:38 +01:00
Enforce and document exact-match-only for signature verification
All checks were successful
check / check (push) Successful in 1m44s
Details
cc50b35ca3 
Add explicit tests proving that HMAC-SHA256 signatures verify against
exact URLs only — no suffix matching, wildcard matching, or partial
matching is supported. A signature for cdn.example.com will not verify
for example.com, images.example.com, or any other host.

Changes:
- signature.go: Add documentation comments on Verify() and
  buildSignatureData() specifying exact-match semantics
- signature_test.go: Add TestSigner_Verify_ExactMatchOnly (14 tamper
  cases covering host, path, query, dimensions, format) and
  TestSigner_Sign_ExactHostInData (verifies suffix-related hosts
  produce distinct signatures)
- service_test.go: Add TestService_ValidateRequest_SignatureExactHostMatch
  (integration test verifying ValidateRequest rejects signatures when
  host differs — parent domain, sibling subdomain, deeper subdomain,
  evil suffix, prefixed host)
- README.md: Document exact-match-only behavior in Signature section

Does NOT modify whitelist.go or any whitelist-related code.
clawbot added the needs-review label 2026-03-20 09:46:26 +01:00
clawbot reviewed 2026-03-20 09:49:09 +01:00
clawbot left a comment
Author
Collaborator
Copy Link

Review: PR #40 — Enforce and document exact-match-only for signature verification

Critical Context Check (re: PR #32, PR #35)

Check Result
whitelist.go NOT modified ✅ Confirmed — not in diff
Changes only to signature code + tests ✅ signature.go, signature_test.go, service_test.go, README.md
No existing tests modified ✅ Only additions, zero deletions in test files
No linter/CI config modified ✅ .golangci.yml untouched

Was HMAC already exact-match?

Yes — verified. The existing buildSignatureData() on main uses req.SourceHost verbatim in fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d", ...). The Verify() method recomputes Sign(req) with the incoming request's fields and does hmac.Equal(). HMAC-SHA256 is inherently exact-match — any change to any component produces a completely different hash. There was never any suffix/wildcard/partial matching in the signature code. The worker's claim is correct.

What the PR adds

  1. Documentation comments on Verify() and buildSignatureData() explicitly stating exact-match semantics
  2. 14 tamper test cases (TestSigner_Verify_ExactMatchOnly) covering host variants (parent domain, sibling subdomain, deeper subdomain, evil suffix, prefixed host), path variants, query variants, dimension and format changes
  3. Distinct-signature test (TestSigner_Sign_ExactHostInData) proving suffix-related hosts produce different signatures
  4. Integration test (TestService_ValidateRequest_SignatureExactHostMatch) through ValidateRequest verifying host-mismatch rejection
  5. README update documenting exact-match-only semantics in the Signature Specification section

Policy Compliance

Policy Status
Docker base images pinned by SHA256 ✅
.golangci.yml unmodified ✅
Tests use standard testing package ✅
No mocks where live behavior expected ✅
make check passes (via docker build) ✅
No secrets in code ✅
Full markdown links for references ✅

Build Result

docker build . — PASS ✅

All tests pass including the new exact-match tests. Lint and fmt checks pass. Binary builds successfully.

Verdict: PASS ✅

This is a clean, well-scoped PR. It correctly identifies that HMAC was already exact-match by nature and adds comprehensive documentation and tests to make that guarantee explicit and regression-proof. No whitelist code was touched. Closes issue #27.

## Review: PR #40 — Enforce and document exact-match-only for signature verification ### Critical Context Check (re: [PR #32](https://git.eeqj.de/sneak/pixa/pulls/32), [PR #35](https://git.eeqj.de/sneak/pixa/pulls/35)) | Check | Result | |-------|--------| | `whitelist.go` NOT modified | ✅ Confirmed — not in diff | | Changes only to signature code + tests | ✅ `signature.go`, `signature_test.go`, `service_test.go`, `README.md` | | No existing tests modified | ✅ Only additions, zero deletions in test files | | No linter/CI config modified | ✅ `.golangci.yml` untouched | ### Was HMAC already exact-match? **Yes — verified.** The existing `buildSignatureData()` on `main` uses `req.SourceHost` verbatim in `fmt.Sprintf("%s:%s:%s:%d:%d:%s:%d", ...)`. The `Verify()` method recomputes `Sign(req)` with the incoming request's fields and does `hmac.Equal()`. HMAC-SHA256 is inherently exact-match — any change to any component produces a completely different hash. There was never any suffix/wildcard/partial matching in the signature code. The worker's claim is correct. ### What the PR adds 1. **Documentation comments** on `Verify()` and `buildSignatureData()` explicitly stating exact-match semantics 2. **14 tamper test cases** (`TestSigner_Verify_ExactMatchOnly`) covering host variants (parent domain, sibling subdomain, deeper subdomain, evil suffix, prefixed host), path variants, query variants, dimension and format changes 3. **Distinct-signature test** (`TestSigner_Sign_ExactHostInData`) proving suffix-related hosts produce different signatures 4. **Integration test** (`TestService_ValidateRequest_SignatureExactHostMatch`) through `ValidateRequest` verifying host-mismatch rejection 5. **README update** documenting exact-match-only semantics in the Signature Specification section ### Policy Compliance | Policy | Status | |--------|--------| | Docker base images pinned by SHA256 | ✅ | | `.golangci.yml` unmodified | ✅ | | Tests use standard `testing` package | ✅ | | No mocks where live behavior expected | ✅ | | `make check` passes (via `docker build`) | ✅ | | No secrets in code | ✅ | | Full markdown links for references | ✅ | ### Build Result `docker build .` — **PASS** ✅ All tests pass including the new exact-match tests. Lint and fmt checks pass. Binary builds successfully. ### Verdict: **PASS** ✅ This is a clean, well-scoped PR. It correctly identifies that HMAC was already exact-match by nature and adds comprehensive documentation and tests to make that guarantee explicit and regression-proof. No whitelist code was touched. Closes [issue #27](https://git.eeqj.de/sneak/pixa/issues/27).
clawbot added merge-ready and removed needs-review labels 2026-03-20 09:49:34 +01:00
sneak was assigned by clawbot 2026-03-20 09:49:34 +01:00
sneak merged commit a50364bfca into main 2026-03-20 23:56:45 +01:00
sneak deleted branch fix/signature-exact-match-only 2026-03-20 23:56:45 +01:00
Sign in to join this conversation.
Reviewers
No Reviewers
clawbot
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label merge-ready
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees sneak
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#40
Delete Branch "fix/signature-exact-match-only"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?