fix: limit cached-read grants to object reads

.claude/rules/transparent-proxy.md

@@ -42,7 +42,8 @@ TAG adds four proxy headers using HMAC-SHA256 over `forwarded_host\ntimestamp\nm
 ## Per-Bucket Authorization
 
 - `AuthzCache` keyed on `(accessKey, bucket)`. Auth for one bucket does not grant access to others.
-- Granted when Tigris returns 2xx with signing keys
+- `AuthzCache` authorization is granted only after a successful upstream client `GetObject` or `HeadObject`; writes and other passthrough operations do not grant cached-read access.
+- Signing keys may be learned from any successful Tigris response carrying the internal header; they prove identity, not read permission.
 - Revoked when Tigris returns 403
 - TTL: 10 minutes (configurable via `TAG_AUTHZ_CACHE_TTL`)
 

README.md

@@ -176,7 +176,7 @@ See [docs/metrics.md](docs/metrics.md) for complete metrics reference.
 
 ## Security
 
-TAG supports transparent proxy mode (default) with local SigV4 validation and per-bucket authorization caching, as well as signing mode with local credential stores.
+TAG supports transparent proxy mode (default) with local SigV4 validation and per-bucket authorization caching, as well as signing mode with local credential stores. In transparent mode, TAG grants cached-read authorization for a bucket only after a successful upstream `GetObject` or `HeadObject`. Successful writes and other passthrough responses do not grant cached-read access.
 
 See [docs/security.md](docs/security.md) for authentication, access control, and security architecture.
 

auth/authz_cache.go

@@ -15,7 +15,7 @@ const DefaultAuthzCacheTTL = 10 * time.Minute
 const maxAuthzCacheSize = 1_000_000
 
 // AuthzCache caches authorization decisions at the (access_key, bucket) level.
-// Entries are granted when Tigris returns a successful response with signing keys,
+// Entries are granted after a client GetObject or HeadObject succeeds upstream,
 // revoked when Tigris returns 403, and expire after a configurable TTL.
 //
 // Uses hashicorp/golang-lru/v2/expirable for TTL-based expiration and bounded size.

docs/security.md

@@ -131,13 +131,13 @@ Request arrives at TAG
 
 ## Authorization Lifecycle
 
-Authorization decisions are cached per `(accessKey, bucket)` pair:
+Authorization decisions are cached per `(accessKey, bucket)` pair. TAG may learn signing keys from any successful response with the internal header. Those keys prove identity, not read permission. TAG grants bucket authorization only after a successful upstream `GetObject` or `HeadObject` response. Successful writes and other passthrough operations do not grant cached-read access.
 
-| Event                                | Action                                    |
-| ------------------------------------ | ----------------------------------------- |
-| Tigris returns 2xx with signing keys | `AuthzCache.Grant(accessKey, bucket)`     |
-| Tigris returns 403                   | `AuthzCache.Revoke(accessKey, bucket)`    |
-| TTL expires (10 min default)         | Entry removed, next request re-authorizes |
+| Event                                              | Action                                    |
+| -------------------------------------------------- | ----------------------------------------- |
+| Upstream 2xx to client `GetObject` or `HeadObject` | `AuthzCache.Grant(accessKey, bucket)`     |
+| Tigris returns 403                                 | `AuthzCache.Revoke(accessKey, bucket)`    |
+| TTL expires (10 min default)                       | Entry removed, next request re-authorizes |
 
 Authorization is strictly per-bucket. A client may have access to some buckets but not others, and TAG enforces this at the cache level.
 

proxy/forwarder_transparent.go

@@ -41,13 +41,51 @@ func (f *transparentForwarder) initInterceptor() {
 }
 
 // interceptResponse is called by base forwarder methods after receiving the
-// upstream response but before headers are sent to the client. It extracts
-// signing keys from successful responses and revokes authZ on 403s.
+// upstream response but before headers are sent to the client. It learns
+// signing keys, grants authorization only for successful object reads, and
+// revokes authZ on 403s.
 func (f *transparentForwarder) interceptResponse(resp *http.Response, originalReq *http.Request) {
 	f.learnSigningKeys(resp, originalReq)
+	f.grantReadAuthorization(resp, originalReq)
 	f.handleAuthzRevocation(resp, originalReq)
 }
 
+// isReadAuthorizationQuery accepts only selectors and authentication metadata
+// known not to select a different S3 operation. The basic object route also
+// receives unrecognized GET/HEAD subresources, so an unknown or malformed
+// query cannot prove that the origin authorized GetObject or HeadObject.
+func isReadAuthorizationQuery(rawQuery string) bool {
+	if rawQuery == "" {
+		return true
+	}
+
+	query, err := url.ParseQuery(rawQuery)
+	if err != nil {
+		return false
+	}
+
+	for key := range query {
+		switch key {
+		case "versionId", "partNumber", "response-cache-control", "response-content-disposition",
+			"response-content-encoding", "response-content-language", "response-content-type", "response-expires",
+			"X-Amz-Checksum-Mode", "x-amz-expected-bucket-owner", "x-amz-request-payer":
+			continue
+		}
+
+		allowedAuthOrSDKParam := false
+		for _, param := range originlessIgnoredParams {
+			if key == param {
+				allowedAuthOrSDKParam = true
+				break
+			}
+		}
+		if !allowedAuthOrSDKParam {
+			return false
+		}
+	}
+	return true
+}
+
 // shallowHeaderCopy gives the forwarded request its own header map while
 // reusing the immutable value slices owned by the client request. Header.Set
 // and Header.Del replace or remove entries, so the values for headers changed
@@ -286,6 +324,7 @@ func (f *transparentForwarder) DoRequestWithCreds(ctx context.Context, r *http.R
 
 // learnSigningKeys extracts and caches derived signing keys from the Tigris response.
 // The signing keys header is always stripped before the response reaches the client.
+// Learning identity keys does not grant authorization to read cached objects.
 func (f *transparentForwarder) learnSigningKeys(resp *http.Response, r *http.Request) {
 	// Always strip the internal header, even when local auth is disabled.
 	headerVal := resp.Header.Get(signingKeysHeader)
@@ -329,7 +368,6 @@ func (f *transparentForwarder) learnSigningKeys(resp *http.Response, r *http.Req
 	}
 
 	bucket, _ := ParseBucketKey(r)
-	f.authzCache.Grant(authInfo.AccessKey, bucket)
 
 	if newKeys > 0 {
 		log.Debug().
@@ -344,6 +382,29 @@ func (f *transparentForwarder) learnSigningKeys(resp *http.Response, r *http.Req
 	metrics.AuthzCacheSize.Set(float64(f.authzCache.Count()))
 }
 
+// grantReadAuthorization records bucket authorization only after an upstream
+// success from a client GetObject or HeadObject. The method, object path, and
+// query classifier reject other operations that can reach the basic object route.
+func (f *transparentForwarder) grantReadAuthorization(resp *http.Response, r *http.Request) {
+	if f.validator == nil || f.keyUnwrapper == nil || f.authzCache == nil ||
+		resp.StatusCode < 200 || resp.StatusCode >= 300 || r == nil ||
+		(r.Method != http.MethodGet && r.Method != http.MethodHead) || !isReadAuthorizationQuery(r.URL.RawQuery) {
+		return
+	}
+
+	authInfo, err := auth.ParseAuthInfo(r)
+	if err != nil {
+		return
+	}
+	bucket, key := ParseBucketKey(r)
+	if bucket == "" || key == "" {
+		return
+	}
+
+	f.authzCache.Grant(authInfo.AccessKey, bucket)
+	metrics.AuthzCacheSize.Set(float64(f.authzCache.Count()))
+}
+
 // handleAuthzRevocation revokes authorization when Tigris returns 403.
 func (f *transparentForwarder) handleAuthzRevocation(resp *http.Response, r *http.Request) {
 	if f.authzCache == nil || resp.StatusCode != http.StatusForbidden {

proxy/forwarder_transparent_test.go

@@ -1,13 +1,21 @@
 package proxy
 
 import (
+	"context"
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rand"
+	"crypto/sha256"
+	"encoding/base64"
+	"encoding/json"
 	"errors"
 	"io"
 	"net/http"
 	"net/http/httptest"
 	"reflect"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/tigrisdata/tag/auth"
 )
@@ -522,3 +530,165 @@ func TestTransparentForwarderSynchronousPathsKeepHeadersPrivate(t *testing.T) {
 		})
 	}
 }
+
+const transparentResponseTestSecret = "response-interceptor-test-secret"
+
+func transparentResponseTestEnvelope(t *testing.T, accessKey string, plaintext []byte) string {
+	t.Helper()
+	hash := sha256.Sum256([]byte(transparentResponseTestSecret))
+	block, err := aes.NewCipher(hash[:])
+	if err != nil {
+		t.Fatal(err)
+	}
+	gcm, err := cipher.NewGCM(block)
+	if err != nil {
+		t.Fatal(err)
+	}
+	nonce := make([]byte, gcm.NonceSize())
+	if _, err := rand.Read(nonce); err != nil {
+		t.Fatal(err)
+	}
+	raw := gcm.Seal(nonce, nonce, plaintext, []byte(accessKey))
+	return base64.StdEncoding.EncodeToString(raw)
+}
+
+func transparentResponseTestRequest(t *testing.T, method, path, accessKey string) *http.Request {
+	t.Helper()
+	request, err := auth.NewRequestSigner("https://upstream.example.com", "us-east-1").SignRequest(
+		context.Background(), method, path, nil, "", accessKey, "client-secret", nil,
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	return request
+}
+
+func TestResponseInterceptorStripsSigningKeysOnFailureAndDisabledAuth(t *testing.T) {
+	const accessKey = "INTERCEPTORKEY"
+	entries, err := json.Marshal([]auth.SigningKeyEntry{{
+		Date:       "20250211",
+		Region:     "auto",
+		SigningKey: strings.Repeat("a", 64),
+	}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	validEnvelope := transparentResponseTestEnvelope(t, accessKey, entries)
+	wrongAADEnvelope := transparentResponseTestEnvelope(t, "OTHERACCESSKEY", entries)
+	corruptRaw, err := base64.StdEncoding.DecodeString(validEnvelope)
+	if err != nil {
+		t.Fatal(err)
+	}
+	corruptRaw[len(corruptRaw)-1] ^= 0x80
+	corruptEnvelope := base64.StdEncoding.EncodeToString(corruptRaw)
+	malformedEnvelope := transparentResponseTestEnvelope(t, accessKey, []byte("{"))
+	emptyEnvelope := transparentResponseTestEnvelope(t, accessKey, []byte("[]"))
+
+	tests := []struct {
+		name     string
+		status   int
+		envelope string
+		authOn   bool
+	}{
+		{name: "wrong AAD", status: http.StatusOK, envelope: wrongAADEnvelope, authOn: true},
+		{name: "corrupt ciphertext", status: http.StatusOK, envelope: corruptEnvelope, authOn: true},
+		{name: "malformed list", status: http.StatusOK, envelope: malformedEnvelope, authOn: true},
+		{name: "empty list", status: http.StatusOK, envelope: emptyEnvelope, authOn: true},
+		{name: "non-2xx response", status: http.StatusForbidden, envelope: validEnvelope, authOn: true},
+		{name: "local authentication disabled", status: http.StatusOK, envelope: validEnvelope, authOn: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			keys := auth.NewDerivedKeyStore(auth.DefaultDerivedKeyTTL)
+			authz := auth.NewAuthzCache(time.Minute)
+			fwd := &transparentForwarder{authzCache: authz}
+			if tt.authOn {
+				unwrapper, err := auth.NewKeyUnwrapper(transparentResponseTestSecret)
+				if err != nil {
+					t.Fatal(err)
+				}
+				fwd.derivedKeyStore = keys
+				fwd.validator = auth.NewRequestValidator(keys)
+				fwd.keyUnwrapper = unwrapper
+			}
+			response := &http.Response{
+				StatusCode: tt.status,
+				Header:     http.Header{signingKeysHeader: []string{tt.envelope}},
+			}
+			fwd.interceptResponse(response, transparentResponseTestRequest(t, http.MethodPost, "/bucket/key?uploads", accessKey))
+
+			if got := response.Header.Get(signingKeysHeader); got != "" {
+				t.Fatalf("internal signing-keys header was not stripped: %q", got)
+			}
+			if keys.HasKey(accessKey) {
+				t.Fatal("failed, empty, non-2xx, or disabled response unexpectedly learned a signing key")
+			}
+			if authz.IsAuthorized(accessKey, "bucket") {
+				t.Fatal("passthrough response unexpectedly granted read authorization")
+			}
+		})
+	}
+}
+
+func TestSuccessfulObjectReadGrantsAuthorizationWithoutSigningKeyHeader(t *testing.T) {
+	const accessKey = "READWITHOUTKEYHEADER"
+	keys := auth.NewDerivedKeyStore(auth.DefaultDerivedKeyTTL)
+	authz := auth.NewAuthzCache(time.Minute)
+	unwrapper, err := auth.NewKeyUnwrapper(transparentResponseTestSecret)
+	if err != nil {
+		t.Fatal(err)
+	}
+	fwd := &transparentForwarder{
+		derivedKeyStore: keys,
+		validator:       auth.NewRequestValidator(keys),
+		keyUnwrapper:    unwrapper,
+		authzCache:      authz,
+	}
+	request := transparentResponseTestRequest(t, http.MethodGet, "/bucket/key", accessKey)
+	response := &http.Response{StatusCode: http.StatusOK, Header: make(http.Header)}
+
+	fwd.interceptResponse(response, request)
+
+	if !authz.IsAuthorized(accessKey, "bucket") {
+		t.Fatal("successful client GetObject did not grant bucket authorization without a signing-keys header")
+	}
+	if keys.HasKey(accessKey) {
+		t.Fatal("read authorization unexpectedly created a signing key")
+	}
+}
+
+func TestReadAuthorizationQueryClassifierAcceptsOnlyObjectReadQueries(t *testing.T) {
+	const presignedReadParams = "X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKID%2Fdate%2Fregion%2Fs3%2Faws4_request&X-Amz-Date=20261005T180039Z&X-Amz-Expires=900&X-Amz-SignedHeaders=host&X-Amz-Signature=signature&X-Amz-Security-Token=token&X-Amz-Checksum-Mode=ENABLED&x-amz-expected-bucket-owner=123456789012&x-amz-request-payer=requester"
+	tests := []struct {
+		name  string
+		query string
+		want  bool
+	}{
+		{name: "plain object read", want: true},
+		{name: "versioned read", query: "versionId=v1", want: true},
+		{name: "part read", query: "partNumber=2", want: true},
+		{name: "response overrides", query: "response-cache-control=no-cache&response-content-disposition=attachment&response-content-encoding=gzip&response-content-language=en&response-content-type=text%2Fplain&response-expires=Wed%2C21%20Oct%202015%2007%3A28%3A00%20GMT", want: true},
+		{name: "presigned GetObject", query: presignedReadParams + "&x-id=GetObject", want: true},
+		{name: "presigned HeadObject", query: presignedReadParams, want: true},
+		{name: "GetObjectAttributes", query: "attributes", want: false},
+		{name: "attribute selector with read selectors", query: "attributes&versionId=v1&x-id=GetObjectAttributes", want: false},
+		{name: "object tagging", query: "tagging", want: false},
+		{name: "object ACL", query: "acl", want: false},
+		{name: "list parts", query: "uploadId=upload", want: false},
+		{name: "object lock retention", query: "retention", want: false},
+		{name: "object lock legal hold", query: "legal-hold", want: false},
+		{name: "torrent", query: "torrent", want: false},
+		{name: "unknown operation", query: "unknown=1", want: false},
+		{name: "malformed query", query: "attributes%ZZ", want: false},
+		{name: "malformed separator", query: "attributes;versionId=v1", want: false},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isReadAuthorizationQuery(tt.query); got != tt.want {
+				t.Fatalf("read authorization query = %v, want %v for %q", got, tt.want, tt.query)
+			}
+		})
+	}
+}