From c4015858faf58b199045a9cf39cb728f5552b7f8 Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 04:58:20 +0000 Subject: [PATCH] Test that a colon cannot move where the path ends and the query begins A URL signed for path /a:b with no query must not verify for path /a with query b:, and the two must not share a cache key. New known-answer vectors pin the signature of a path with ":", a query with ":", and a "%" in path and query, with "%" written as "%25" and ":" as "%3A" in the signed data. These fail until the fix for https://git.eeqj.de/sneak/pixa/issues/247 lands. Model: opus-5-5 --- internal/imgcache/storage_internal_test.go | 56 ++++++++++ internal/signature/escaping_test.go | 113 +++++++++++++++++++++ 2 files changed, 169 insertions(+) create mode 100644 internal/signature/escaping_test.go diff --git a/internal/imgcache/storage_internal_test.go b/internal/imgcache/storage_internal_test.go index c57e1ee..1d3996d 100644 --- a/internal/imgcache/storage_internal_test.go +++ b/internal/imgcache/storage_internal_test.go @@ -407,6 +407,62 @@ func TestCacheKey(t *testing.T) { } } +// TestCacheKey_ColonOrPercentInPathOrQuery checks that two different source +// URLs whose path or query holds ":" or "%" get different keys. +func TestCacheKey_ColonOrPercentInPathOrQuery(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + path1, query1 string + path2, query2 string + }{ + { + name: "colon moved from path to query", + path1: "/a:b", query1: "", + path2: "/a", query2: "b:", + }, + { + name: "colon in path and its percent-encoding", + path1: "/a:b", query1: "", + path2: "/a%3Ab", query2: "", + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + key1 := CacheKey(&ImageRequest{ + SourceHost: testHostCDN, + SourcePath: tt.path1, + SourceQuery: tt.query1, + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + Quality: 85, + FitMode: FitCover, + }) + key2 := CacheKey(&ImageRequest{ + SourceHost: testHostCDN, + SourcePath: tt.path2, + SourceQuery: tt.query2, + Size: Size{Width: 800, Height: 600}, + Format: FormatWebP, + Quality: 85, + FitMode: FitCover, + }) + + t.Logf("CacheKey() of path %q query %q = %s", tt.path1, tt.query1, key1) + t.Logf("CacheKey() of path %q query %q = %s", tt.path2, tt.query2, key2) + + if key1 == key2 { + t.Errorf("CacheKey() gave path %q query %q and path %q query %q "+ + "the same key", tt.path1, tt.query1, tt.path2, tt.query2) + } + }) + } +} + // TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write // that fails is logged at warn and the store still succeeds. func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) { diff --git a/internal/signature/escaping_test.go b/internal/signature/escaping_test.go new file mode 100644 index 0000000..550805c --- /dev/null +++ b/internal/signature/escaping_test.go @@ -0,0 +1,113 @@ +package signature_test + +import ( + "errors" + "testing" + "time" + + "sneak.berlin/go/pixa/internal/signature" +) + +// TestSigner_Verify_ColonCannotMovePathQueryBoundary checks that a URL +// signed for path "/a:b" with no query does not verify for path "/a" with +// query "b:", a different resource on the same host. +func TestSigner_Verify_ColonCannotMovePathQueryBoundary(t *testing.T) { + t.Parallel() + + signer := signature.New(goldenSigningKey) + + signed := &signature.Request{ + SourceHost: testHost, + SourcePath: "/a:b", + Width: 800, + Height: 600, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + Expires: time.Now().Add(1 * time.Hour), + } + signed.Signature = signer.Sign(signed) + + replayed := *signed + replayed.SourcePath = "/a" + replayed.SourceQuery = "b:" + + t.Logf("signature for path %q query %q: %s", + signed.SourcePath, signed.SourceQuery, signed.Signature) + t.Logf("signature for path %q query %q: %s", + replayed.SourcePath, replayed.SourceQuery, signer.Sign(&replayed)) + + err := signer.Verify(&replayed) + if !errors.Is(err, signature.ErrInvalid) { + t.Errorf("Verify() of path %q query %q with the signature for "+ + "path %q = %v, want %v", + replayed.SourcePath, replayed.SourceQuery, signed.SourcePath, + err, signature.ErrInvalid) + } +} + +// TestSigner_EscapedGoldenVectors pins the signature of requests whose path +// or query holds ":" or "%", which are signed with "%" written as "%25" and +// ":" as "%3A". The expected values were computed once, outside pixa, from +// the signed data in each comment. +func TestSigner_EscapedGoldenVectors(t *testing.T) { + t.Parallel() + + signer := signature.New(goldenSigningKey) + + vectors := []struct { + name string + path string + query string + wantSignature string + }{ + { + name: "colon in path", + path: "/photos/a:b.jpg", + // Signed data: + // "cdn.example.com:/photos/a%3Ab.jpg::800:600:webp:1704067200:85:cover" + wantSignature: "WnEpZ7bQp1Zvw8WDatlj2vu-4wYKWE3BFBizV8m__6k=", + }, + { + name: "colon in query", + path: testPath, + query: "t=1:2", + // Signed data: + // "cdn.example.com:/photos/cat.jpg:t=1%3A2:800:600:webp:1704067200:85:cover" + wantSignature: "85kRoN8hhr4cCUkEUjNvUsvVYKjYYfWdeffx5TjJjqA=", + }, + { + name: "percent in path and query", + path: "/photos/a%20b.jpg", + query: "q=50%", + // Signed data: + // "cdn.example.com:/photos/a%2520b.jpg:q=50%25:800:600:webp:1704067200:85:cover" + wantSignature: "MjDiiRdgXMrsjqPMsENtD4aG1hp1e-a6zy67vKTX4CQ=", + }, + } + + for _, tt := range vectors { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + req := &signature.Request{ + SourceHost: testHost, + SourcePath: tt.path, + SourceQuery: tt.query, + Width: 800, + Height: 600, + Format: testFormatWebP, + Quality: 85, + FitMode: testFitCover, + Expires: time.Unix(goldenExpiresUnix, 0), + } + + gotSignature := signer.Sign(req) + t.Logf("Sign() of path %q query %q = %s", tt.path, tt.query, gotSignature) + + if gotSignature != tt.wantSignature { + t.Errorf("Sign() = %q, want %q", gotSignature, tt.wantSignature) + } + }) + } +}