From a9819affc4b8b12d569a37a2295c83b1f7a7dd85 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Mon, 14 Sep 2026 20:16:01 +0530
Subject: [PATCH 01/25] feat(agent-vault): enforce method and path rules,
 inject headers, substitute placeholders

The resolve payload now carries a service's allowed methods and path prefixes,
its extra headers, and its placeholder substitutions.

The policy check runs on the request exactly as it arrived, above the plaintext
refusal so a rule holds on http:// too, and answers a violation with a 403.
Path matching never decodes: it compares the prefix against EscapedPath byte for
byte at a segment boundary, and refuses any path carrying a . or .. segment, an
empty segment, a ; or \, or an escape decoding to one of those, to a control
byte, or to bytes that are not valid UTF-8. That last test is what lets a real
non-ASCII path through while still refusing the overlong %c0%ae.

Substitutions run before the credential so an injected value can never itself be
rewritten, and custom headers last. A path substitution rewrites the path after
it was authorised, so a path-restricted service re-checks what actually goes on
the wire; that refusal carries no path, because by then the path holds the real
credential and the error text is both the 403 body and the log line. The log
gains a substituted field naming the surfaces actually rewritten, since the
logged path is always the agent's own and a placeholder that matched nothing
would otherwise look identical to one that fired.
---
 packages/agentvault/cache.go                  |  19 +-
 packages/agentvault/policy.go                 | 158 ++++++++
 packages/agentvault/policy_test.go            | 240 +++++++++++++
 packages/agentvault/proxy.go                  |  74 +++-
 packages/agentvault/proxy_policy_test.go      | 339 ++++++++++++++++++
 packages/agentvault/resolve.go                |  81 ++++-
 packages/agentvault/rewrite.go                | 175 +++++++++
 .../rewrite_transformations_test.go           | 162 +++++++++
 packages/api/agent_vault.go                   |  27 +-
 9 files changed, 1254 insertions(+), 21 deletions(-)
 create mode 100644 packages/agentvault/policy.go
 create mode 100644 packages/agentvault/policy_test.go
 create mode 100644 packages/agentvault/proxy_policy_test.go
 create mode 100644 packages/agentvault/rewrite_transformations_test.go

diff --git a/packages/agentvault/cache.go b/packages/agentvault/cache.go
index b4f71ac7..a94770f1 100644
--- a/packages/agentvault/cache.go
+++ b/packages/agentvault/cache.go
@@ -39,12 +39,29 @@ type credential struct {
 	password     []byte
 }
 
+type customHeader struct {
+	name   string
+	prefix string
+	value  []byte
+}
+
+type substitution struct {
+	placeholder string
+	surfaces    map[string]bool
+	value       []byte
+}
+
 type resolvedService struct {
 	id               string
 	name             string
 	accessBundleName string
 	hostPatterns     []hostPattern
-	credential       credential
+	// A nil map means every method is allowed; an empty slice of prefixes means every path.
+	allowedMethods      map[string]bool
+	allowedPathPrefixes []string
+	credential          credential
+	headers             []customHeader
+	substitutions       []substitution
 }
 
 type sessionEntry struct {
diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
new file mode 100644
index 00000000..1fc435c2
--- /dev/null
+++ b/packages/agentvault/policy.go
@@ -0,0 +1,158 @@
+package agentvault
+
+import (
+	"errors"
+	"fmt"
+	"net/http"
+	"strings"
+	"unicode/utf8"
+)
+
+// errPolicyBlocked is the sentinel for a service's own method and path rules, distinct from errHostBlocked,
+// which is the proxy-wide traffic policy. Both render as a 403 whose body is err.Error().
+var errPolicyBlocked = errors.New("blocked by service policy")
+
+// checkServicePolicy runs on the request exactly as it arrived, before anything rewrites it, and before the
+// plaintext refusal that nils a match: a restriction has to hold on http:// too, not only where a credential
+// would have been attached.
+func checkServicePolicy(svc *resolvedService, req *http.Request) error {
+	if !svc.allowsMethod(req.Method) {
+		return fmt.Errorf("service %q does not allow %s: %w", svc.name, req.Method, errPolicyBlocked)
+	}
+	if len(svc.allowedPathPrefixes) > 0 {
+		path := requestPath(req)
+		if !pathAllowed(path, svc.allowedPathPrefixes) {
+			return fmt.Errorf("service %q does not allow path %q: %w", svc.name, truncatePath(path), errPolicyBlocked)
+		}
+	}
+	return nil
+}
+
+// A nil map is every method. The set is built upper-case, and the comparison folds the request's method the
+// same way, so a client sending "get" is judged on GET rather than silently blocked.
+func (s *resolvedService) allowsMethod(method string) bool {
+	if s.allowedMethods == nil {
+		return true
+	}
+	return s.allowedMethods[strings.ToUpper(method)]
+}
+
+// EscapedPath is byte-for-byte what Request.write puts on the wire (RequestURI() returns it, and forward only
+// rewrites Scheme, Host and RequestURI), so this judges exactly what the upstream will receive.
+func requestPath(req *http.Request) string {
+	path := req.URL.EscapedPath()
+	if path == "" {
+		// OPTIONS * arrives as "*" and is left alone; a genuinely empty path is the root.
+		if req.URL.Opaque != "" {
+			return req.URL.Opaque
+		}
+		return "/"
+	}
+	return path
+}
+
+func truncatePath(path string) string {
+	if len(path) > maxLoggedPathLen {
+		return path[:maxLoggedPathLen] + "...[truncated]"
+	}
+	return path
+}
+
+// pathAllowed never decodes. Anything whose meaning would depend on how the upstream normalises it is refused
+// outright, so the prefix comparison below is a plain byte comparison and the filter can only ever allow a
+// path every reader agrees on. Prefixes carry none of these characters by grammar.
+func pathAllowed(escaped string, prefixes []string) bool {
+	if isAmbiguousPath(escaped) {
+		return false
+	}
+	for _, prefix := range prefixes {
+		if prefix == "/" {
+			return true
+		}
+		if !strings.HasPrefix(escaped, prefix) {
+			continue
+		}
+		// Whole segments only, so /repos does not cover /repositories.
+		if rest := escaped[len(prefix):]; rest == "" || rest[0] == '/' {
+			return true
+		}
+	}
+	return false
+}
+
+func isAmbiguousPath(escaped string) bool {
+	// ';' because Tomcat, Jetty and Spring strip ;params per segment before normalising, so /repos/..;/admin
+	// resolves to /admin upstream while reading as an ordinary segment here. '\' because Go treats it as a
+	// path byte and IIS and .NET read it as a separator.
+	if strings.ContainsAny(escaped, ";\\") {
+		return true
+	}
+	if hasUnsafeEscape(escaped) {
+		return true
+	}
+	for _, segment := range strings.Split(escaped, "/") {
+		if segment == "." || segment == ".." {
+			return true
+		}
+	}
+	// An empty segment: /a//b normalises differently per server. A leading // is covered too; it could only
+	// ever fail the prefix comparison anyway, but judging it here keeps the rule one sentence.
+	return strings.Contains(escaped, "//")
+}
+
+// Judges the percent-escapes in a path. An escape is unsafe when it decodes to a separator, a dot, a
+// control byte, or to a byte sequence that is not valid UTF-8.
+//
+// The UTF-8 check is what lets a real non-ASCII path through while still refusing the attack it protects
+// against. `%c3%a9` is a correctly encoded 'é' and decodes to one rune; `%c0%ae` is an overlong encoding
+// of '.', which Go decodes to RuneError and some servers read as a dot. Rejecting every byte >= 0x80
+// would catch the second but also break every API that carries a filename or a user string in its path.
+// The rest (%2e, %2f, %5c, the double-encoded %252e, and the null-truncation ..%00) fall out of the
+// decoded-byte switch. %20 still works.
+func hasUnsafeEscape(escaped string) bool {
+	decoded := make([]byte, 0, len(escaped))
+	sawEscape := false
+
+	for i := 0; i < len(escaped); i++ {
+		if escaped[i] != '%' {
+			decoded = append(decoded, escaped[i])
+			continue
+		}
+		if i+2 >= len(escaped) {
+			// A truncated escape is not something we can judge either.
+			return true
+		}
+		hi, hiOk := unhex(escaped[i+1])
+		lo, loOk := unhex(escaped[i+2])
+		if !hiOk || !loOk {
+			return true
+		}
+		b := hi<<4 | lo
+		if b < 0x20 || b == 0x7f {
+			return true
+		}
+		switch b {
+		case '.', '/', '\\', ';', '%':
+			return true
+		}
+		decoded = append(decoded, b)
+		sawEscape = true
+		i += 2
+	}
+
+	// Only escaped input can carry an overlong or truncated sequence; an unescaped path is whatever the
+	// client put on the wire and is compared byte for byte anyway.
+	return sawEscape && !utf8.Valid(decoded)
+}
+
+func unhex(c byte) (byte, bool) {
+	switch {
+	case c >= '0' && c <= '9':
+		return c - '0', true
+	case c >= 'a' && c <= 'f':
+		return c - 'a' + 10, true
+	case c >= 'A' && c <= 'F':
+		return c - 'A' + 10, true
+	}
+	return 0, false
+}
diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
new file mode 100644
index 00000000..b1f51be4
--- /dev/null
+++ b/packages/agentvault/policy_test.go
@@ -0,0 +1,240 @@
+package agentvault
+
+import (
+	"errors"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func serviceWithPolicy(methods []string, prefixes []string) *resolvedService {
+	return &resolvedService{
+		name:                "github",
+		allowedMethods:      toMethodSet(methods),
+		allowedPathPrefixes: toPathPrefixes(prefixes),
+	}
+}
+
+func requestTo(t *testing.T, method, target string) *http.Request {
+	t.Helper()
+	req, err := http.NewRequest(method, "https://api.github.com"+target, nil)
+	if err != nil {
+		t.Fatalf("building request: %v", err)
+	}
+	return req
+}
+
+func TestMethodPolicy(t *testing.T) {
+	t.Run("a nil set allows every method", func(t *testing.T) {
+		svc := serviceWithPolicy(nil, nil)
+		for _, method := range []string{"GET", "POST", "DELETE", "PROPFIND"} {
+			if err := checkServicePolicy(svc, requestTo(t, method, "/x")); err != nil {
+				t.Fatalf("%s should be allowed: %v", method, err)
+			}
+		}
+	})
+
+	t.Run("only the listed methods pass", func(t *testing.T) {
+		svc := serviceWithPolicy([]string{"GET", "HEAD"}, nil)
+		if err := checkServicePolicy(svc, requestTo(t, "GET", "/x")); err != nil {
+			t.Fatalf("GET should be allowed: %v", err)
+		}
+		err := checkServicePolicy(svc, requestTo(t, "POST", "/x"))
+		if !errors.Is(err, errPolicyBlocked) {
+			t.Fatalf("POST should be blocked, got %v", err)
+		}
+		// The body is err.Error(), so the service and the method both have to be in it.
+		if !strings.Contains(err.Error(), `service "github" does not allow POST`) {
+			t.Fatalf("unhelpful message: %q", err.Error())
+		}
+	})
+
+	t.Run("a lower-case method is folded rather than blocked", func(t *testing.T) {
+		svc := serviceWithPolicy([]string{"GET"}, nil)
+		req := requestTo(t, "GET", "/x")
+		req.Method = "get"
+		if err := checkServicePolicy(svc, req); err != nil {
+			t.Fatalf("get should fold to GET: %v", err)
+		}
+	})
+}
+
+func TestPathPolicy(t *testing.T) {
+	svc := serviceWithPolicy(nil, []string{"/repos"})
+
+	allowed := []string{"/repos", "/repos/", "/repos/octo/hello", "/repos/a%20b"}
+	for _, path := range allowed {
+		t.Run("allows "+path, func(t *testing.T) {
+			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); err != nil {
+				t.Fatalf("%s should be allowed: %v", path, err)
+			}
+		})
+	}
+
+	// Each of these reads as inside /repos to a naive prefix check but resolves elsewhere on some upstream.
+	blocked := []string{
+		"/repositories",
+		"/repo",
+		"/admin",
+		"/repos/../admin",
+		"/repos/./x",
+		"//repos/x",
+		"/repos/%2e%2e/admin",
+		"/repos/%2E%2E/admin",
+		"/admin/%2e%2e/repos/x",
+		"/repos/%252e%252e/admin",
+		"/repos/%c0%ae%c0%ae/admin",
+		"/repos/..;/admin",
+		"/repos;x/y",
+		"/repos/%2fadmin",
+		"/repos%5cx",
+	}
+	for _, path := range blocked {
+		t.Run("blocks "+path, func(t *testing.T) {
+			req := requestTo(t, "GET", "/placeholder")
+			// Set the target verbatim so Go's URL parsing cannot normalise the case away before we see it.
+			req.URL.Path = ""
+			req.URL.RawPath = ""
+			req.URL.Opaque = ""
+			parsed := requestTo(t, "GET", path)
+			req.URL = parsed.URL
+			err := checkServicePolicy(svc, req)
+			if !errors.Is(err, errPolicyBlocked) {
+				t.Fatalf("%s should be blocked, got %v", path, err)
+			}
+		})
+	}
+
+	t.Run("a backslash is blocked even where Go keeps it literal", func(t *testing.T) {
+		req := requestTo(t, "GET", "/repos")
+		req.URL.Path = `/repos/\..\admin`
+		req.URL.RawPath = ""
+		if err := checkServicePolicy(svc, req); !errors.Is(err, errPolicyBlocked) {
+			t.Fatalf("backslash traversal should be blocked, got %v", err)
+		}
+	})
+
+	t.Run("an unrestricted service is untouched by any of it", func(t *testing.T) {
+		open := serviceWithPolicy(nil, nil)
+		for _, path := range blocked {
+			req := requestTo(t, "GET", path)
+			if err := checkServicePolicy(open, req); err != nil {
+				t.Fatalf("%s should pass on an unrestricted service: %v", path, err)
+			}
+		}
+	})
+
+	t.Run("prefix / matches everything", func(t *testing.T) {
+		root := serviceWithPolicy(nil, []string{"/"})
+		if err := checkServicePolicy(root, requestTo(t, "GET", "/anything/at/all")); err != nil {
+			t.Fatalf("/ should match: %v", err)
+		}
+	})
+
+	t.Run("a trailing slash on the prefix is normalised away", func(t *testing.T) {
+		trailing := serviceWithPolicy(nil, []string{"/repos/"})
+		if err := checkServicePolicy(trailing, requestTo(t, "GET", "/repos/octo")); err != nil {
+			t.Fatalf("/repos/ should cover /repos/octo: %v", err)
+		}
+	})
+
+	t.Run("an empty path reads as the root", func(t *testing.T) {
+		root := serviceWithPolicy(nil, []string{"/"})
+		req := requestTo(t, "GET", "/")
+		req.URL.Path = ""
+		if err := checkServicePolicy(root, req); err != nil {
+			t.Fatalf("an empty path should read as /: %v", err)
+		}
+	})
+}
+
+func TestControlByteEscapesAreRefused(t *testing.T) {
+	svc := serviceWithPolicy(nil, []string{"/repos"})
+	// ..%00 is the null-truncation traversal: an upstream that decodes then truncates at NUL, or strips
+	// control bytes before normalising, reads this as /repos/.. and resolves outside the prefix.
+	for _, path := range []string{"/repos/..%00/admin", "/repos/%00../admin", "/repos/x%09y", "/repos/x%7f"} {
+		t.Run(path, func(t *testing.T) {
+			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); !errors.Is(err, errPolicyBlocked) {
+				t.Fatalf("%s should be blocked, got %v", path, err)
+			}
+		})
+	}
+}
+
+func TestWireMappingFailsClosed(t *testing.T) {
+	t.Run("nil stays unrestricted", func(t *testing.T) {
+		if toMethodSet(nil) != nil || toPathPrefixes(nil) != nil {
+			t.Fatal("a nil list must stay nil, which every caller reads as unrestricted")
+		}
+	})
+
+	t.Run("an empty restriction allows nothing", func(t *testing.T) {
+		methods := toMethodSet([]string{})
+		if methods == nil || len(methods) != 0 {
+			t.Fatalf("an empty method list must restrict, got %v", methods)
+		}
+
+		// A list whose entries are all blank must not collapse to "unrestricted".
+		prefixes := toPathPrefixes([]string{"  "})
+		if len(prefixes) == 0 {
+			t.Fatal("an empty path prefix list must restrict, not fall through to unrestricted")
+		}
+		svc := &resolvedService{name: "s", allowedPathPrefixes: prefixes}
+		if err := checkServicePolicy(svc, requestTo(t, "GET", "/anything")); !errors.Is(err, errPolicyBlocked) {
+			t.Fatalf("expected a block, got %v", err)
+		}
+	})
+}
+
+func TestNonAsciiPathsAreJudgedByUtf8Validity(t *testing.T) {
+	svc := serviceWithPolicy(nil, []string{"/repos"})
+
+	// Correctly encoded UTF-8 is an ordinary path: é, 日本語, and an emoji all reach the upstream.
+	allowed := []string{
+		"/repos/owner/repo/contents/caf%C3%A9.md",
+		"/repos/%E6%97%A5%E6%9C%AC%E8%AA%9E",
+		"/repos/a%20b",
+		"/repos/%F0%9F%94%91",
+	}
+	for _, path := range allowed {
+		t.Run("allows "+path, func(t *testing.T) {
+			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); err != nil {
+				t.Fatalf("%s should be allowed: %v", path, err)
+			}
+		})
+	}
+
+	// Overlong and malformed sequences stay blocked: %c0%ae is an overlong '.', which some servers
+	// normalise as a traversal segment.
+	blocked := []string{
+		"/repos/%c0%ae%c0%ae/admin",
+		"/repos/%c0%af",
+		"/repos/%e0%80%ae",
+		"/repos/%ff",
+		"/repos/%c3",
+	}
+	for _, path := range blocked {
+		t.Run("blocks "+path, func(t *testing.T) {
+			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); !errors.Is(err, errPolicyBlocked) {
+				t.Fatalf("%s should be blocked, got %v", path, err)
+			}
+		})
+	}
+}
+
+func TestAnExplicitRootPrefixMatchesEverythingAnUnrestrictedServiceWould(t *testing.T) {
+	// Setting "/" should mean the same as setting no prefix at all, which was not true while every
+	// high byte was refused outright.
+	root := serviceWithPolicy(nil, []string{"/"})
+	open := serviceWithPolicy(nil, nil)
+
+	for _, path := range []string{"/anything", "/repos/caf%C3%A9.md", "/a%20b"} {
+		t.Run(path, func(t *testing.T) {
+			rootErr := checkServicePolicy(root, requestTo(t, "GET", path))
+			openErr := checkServicePolicy(open, requestTo(t, "GET", path))
+			if (rootErr == nil) != (openErr == nil) {
+				t.Fatalf("prefix / and no prefix disagree on %s: %v vs %v", path, rootErr, openErr)
+			}
+		})
+	}
+}
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index f42049f6..0d5682ac 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -357,7 +357,7 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 		reqPath = reqPath[:maxLoggedPathLen] + "...[truncated]"
 	}
 
-	resp, matched, err := ps.forward(r, scheme, hostname, port, sessionToken)
+	resp, matched, outcome, err := ps.forward(r, scheme, hostname, port, sessionToken)
 
 	// The body is fixed text per outcome, never err.Error(): an APIError carries the control-plane URL and
 	// request id, and a dial error names the upstream address. The detail goes on the log line below.
@@ -366,7 +366,7 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 	status := 0
 	body := ""
 	switch {
-	case errors.Is(err, errHostBlocked):
+	case errors.Is(err, errHostBlocked), errors.Is(err, errPolicyBlocked):
 		decision, status, body = decisionBlocked, http.StatusForbidden, err.Error()
 	case isProxyTokenRejected(err):
 		decision, status, body = decisionError, http.StatusServiceUnavailable, proxyRevokedBody
@@ -376,8 +376,9 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 		decision, status, body = decisionError, http.StatusBadGateway, "failed to resolve the session"
 	case err != nil:
 		decision, status, body = decisionError, http.StatusBadGateway, "failed to reach the upstream"
-	// brokered means a credential went out, not merely that a service matched.
-	case matched != nil && matched.credential.kind != credentialPassthrough:
+	// brokered means something was attached or rewritten, not merely that a service matched. A pass-through
+	// service carrying custom headers or substitutions counts.
+	case outcome.brokered:
 		decision, status = decisionBrokered, resp.StatusCode
 	default:
 		status = resp.StatusCode
@@ -400,6 +401,11 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 	if matched != nil {
 		event = event.Str("service", matched.name).Str("accessBundle", matched.accessBundleName)
 	}
+	// Names the surfaces a placeholder was actually swapped in. Without it a substitution that matched
+	// nothing is indistinguishable from one that did: the logged path is the agent's either way.
+	if len(outcome.substituted) > 0 {
+		event = event.Strs("substituted", outcome.substituted)
+	}
 	if err != nil {
 		event = event.Err(err)
 	}
@@ -441,16 +447,33 @@ func (ps *proxyServer) blocksOffBundle(matched *resolvedService, hostname, port
 	return matched == nil && ps.currentConfig().TrafficPolicy == TrafficPolicyBundleHosts && !ps.isAllowedHost(hostname, port)
 }
 
-func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessionToken string) (*http.Response, *resolvedService, error) {
+// What forward did to the request, for the log line. `brokered` is wider than "a credential went out": a
+// pass-through service carrying custom headers or substitutions is still brokering something.
+type forwardOutcome struct {
+	brokered    bool
+	substituted []string
+}
+
+func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessionToken string) (*http.Response, *resolvedService, forwardOutcome, error) {
+	var outcome forwardOutcome
+
 	services, err := ps.cache.get(sessionToken)
 	if err != nil {
-		return nil, nil, fmt.Errorf("%w: %w", errSessionResolve, err)
+		return nil, nil, outcome, fmt.Errorf("%w: %w", errSessionResolve, err)
 	}
 
 	matched := bestMatch(services, hostname, port)
 
 	if ps.blocksOffBundle(matched, hostname, port) {
-		return nil, nil, fmt.Errorf("no service covers host %q: %w", hostname, errHostBlocked)
+		return nil, nil, outcome, fmt.Errorf("no service covers host %q: %w", hostname, errHostBlocked)
+	}
+
+	// Judged on the request as it arrived, and above the plaintext refusal below, so a method or path rule
+	// holds on http:// too rather than only where a credential would have been attached.
+	if matched != nil {
+		if err := checkServicePolicy(matched, req); err != nil {
+			return nil, matched, outcome, err
+		}
 	}
 
 	req.URL.Scheme = scheme
@@ -472,15 +495,46 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 				Msg("agent-vault: refusing to attach a credential over plaintext http")
 			matched = nil
 		} else {
-			injectCredential(req, &matched.credential)
+			// Substitutions run before the credential so an injected real value can never itself be rewritten,
+			// and custom headers last so they are not clobbered by it.
+			outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
+			outcome.brokered = injectCredential(req, &matched.credential)
+			if injectHeaders(req, matched.headers) {
+				outcome.brokered = true
+			}
+			if len(outcome.substituted) > 0 {
+				outcome.brokered = true
+			}
+
+			// A path-surface substitution rewrites the path after the check above, so a restricted service
+			// re-checks what actually goes on the wire.
+			if len(matched.allowedPathPrefixes) > 0 && containsSurface(outcome.substituted, surfacePath) {
+				if !pathAllowed(requestPath(req), matched.allowedPathPrefixes) {
+					// The path now carries the real credential, so it must not reach the body or the log.
+					// Every other refusal in this file is fixed text for the same reason.
+					return nil, matched, outcome, fmt.Errorf(
+						"service %q does not allow the path this request substitutes to: %w",
+						matched.name, errPolicyBlocked,
+					)
+				}
+			}
 		}
 	}
 
 	resp, err := ps.transport.RoundTrip(req)
 	if err != nil {
-		return nil, matched, err
+		return nil, matched, outcome, err
 	}
-	return resp, matched, nil
+	return resp, matched, outcome, nil
+}
+
+func containsSurface(surfaces []string, target string) bool {
+	for _, surface := range surfaces {
+		if surface == target {
+			return true
+		}
+	}
+	return false
 }
 
 func (ps *proxyServer) isAllowedHost(hostname, port string) bool {
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
new file mode 100644
index 00000000..62ee2066
--- /dev/null
+++ b/packages/agentvault/proxy_policy_test.go
@@ -0,0 +1,339 @@
+package agentvault
+
+import (
+	"bytes"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
+)
+
+// What the upstream actually received, so the assertions are about the wire rather than our own structs.
+type echoed struct {
+	Method  string              `json:"method"`
+	Path    string              `json:"path"`
+	Query   string              `json:"query"`
+	Headers map[string][]string `json:"headers"`
+	Body    string              `json:"body"`
+}
+
+type fixedResolver struct{ services []*resolvedService }
+
+func (r fixedResolver) resolve(string) (*resolveResult, error) {
+	return &resolveResult{SessionID: "s1", Services: r.services}, nil
+}
+
+// Stands up the whole path an agent's request takes: CONNECT to the proxy, TLS terminated by the proxy's
+// own CA, policy and injection applied, then forwarded over TLS to a real upstream that echoes what it got.
+// The upstream is addressed as 127.0.0.1, which is what httptest's certificate carries and what mintLeaf
+// puts in an IP SAN, so both TLS legs verify and the CONNECT target the proxy dials is the upstream itself.
+// Returns the client and the service's host, ready to build a URL from.
+func newPolicyFixture(t *testing.T, build func(host string) *resolvedService) (*http.Client, string) {
+	t.Helper()
+
+	upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(echoed{
+			Method:  r.Method,
+			Path:    r.URL.EscapedPath(),
+			Query:   r.URL.RawQuery,
+			Headers: r.Header,
+			Body:    string(body),
+		})
+	}))
+	t.Cleanup(upstream.Close)
+
+	upstreamURL, err := url.Parse(upstream.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	host := "127.0.0.1:" + upstreamURL.Port()
+
+	upstreamPool := x509.NewCertPool()
+	upstreamPool.AddCert(upstream.Certificate())
+
+	key, cert, err := generateRootCa()
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	transport := newUpstreamTransport()
+	// The proxy has to trust the httptest upstream's self-signed certificate to reach it.
+	transport.TLSClientConfig = &tls.Config{RootCAs: upstreamPool}
+
+	ps := &proxyServer{transport: transport, ca: newCaManager(key, cert)}
+	ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost})
+	ps.cache = newSessionCache(fixedResolver{services: []*resolvedService{build(host)}}, ps.pollInterval)
+
+	front := httptest.NewServer(http.HandlerFunc(ps.dispatch))
+	t.Cleanup(front.Close)
+
+	clientPool := x509.NewCertPool()
+	clientPool.AddCert(cert)
+	proxyURL, _ := url.Parse(front.URL)
+	proxyURL.User = url.UserPassword(ProxyAuthUsername, "agv_tok")
+
+	client := &http.Client{
+		Transport: &http.Transport{
+			Proxy:           http.ProxyURL(proxyURL),
+			TLSClientConfig: &tls.Config{RootCAs: clientPool},
+		},
+	}
+	return client, host
+}
+
+func policyService(host string, methods, prefixes []string, headers []customHeader, subs []substitution) *resolvedService {
+	return &resolvedService{
+		name:                "github",
+		accessBundleName:    "bundle",
+		hostPatterns:        parseHostPatterns(host),
+		allowedMethods:      toMethodSet(methods),
+		allowedPathPrefixes: toPathPrefixes(prefixes),
+		credential:          credential{kind: credentialPassthrough},
+		headers:             headers,
+		substitutions:       subs,
+	}
+}
+
+func do(t *testing.T, c *http.Client, method, target, body string) (int, string) {
+	t.Helper()
+	var reader io.Reader
+	if body != "" {
+		reader = strings.NewReader(body)
+	}
+	req, err := http.NewRequest(method, target, reader)
+	if err != nil {
+		t.Fatal(err)
+	}
+	resp, err := c.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	payload, _ := io.ReadAll(resp.Body)
+	return resp.StatusCode, strings.TrimSpace(string(payload))
+}
+
+func decodeEcho(t *testing.T, payload string) echoed {
+	t.Helper()
+	var got echoed
+	if err := json.Unmarshal([]byte(payload), &got); err != nil {
+		t.Fatalf("upstream did not echo JSON (%v): %s", err, payload)
+	}
+	return got
+}
+
+func TestMethodPolicyThroughTheTunnel(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, []string{"GET"}, nil, nil, nil)
+	})
+
+	status, body := do(t, client, "GET", fmt.Sprintf("https://%s/anything", host), "")
+	if status != http.StatusOK {
+		t.Fatalf("GET should reach the upstream, got %d: %s", status, body)
+	}
+	if got := decodeEcho(t, body); got.Method != "GET" {
+		t.Fatalf("upstream saw %q", got.Method)
+	}
+
+	status, body = do(t, client, "POST", fmt.Sprintf("https://%s/anything", host), "x")
+	if status != http.StatusForbidden {
+		t.Fatalf("POST should be refused, got %d: %s", status, body)
+	}
+	if !strings.Contains(body, `service "github" does not allow POST`) {
+		t.Fatalf("unhelpful 403 body: %q", body)
+	}
+}
+
+func TestPathPolicyThroughTheTunnel(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, []string{"/repos"}, nil, nil)
+	})
+
+	status, body := do(t, client, "GET", fmt.Sprintf("https://%s/repos/octo/hello", host), "")
+	if status != http.StatusOK {
+		t.Fatalf("an allowed path should reach the upstream, got %d: %s", status, body)
+	}
+
+	for _, path := range []string{"/repositories", "/admin", "/repos/%2e%2e/admin"} {
+		status, body = do(t, client, "GET", fmt.Sprintf("https://%s%s", host, path), "")
+		if status != http.StatusForbidden {
+			t.Fatalf("%s should be refused, got %d: %s", path, status, body)
+		}
+		if !strings.Contains(body, "blocked by service policy") {
+			t.Fatalf("%s: unhelpful 403 body: %q", path, body)
+		}
+	}
+}
+
+func TestCustomHeadersReachTheUpstream(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, nil, []customHeader{
+			{name: "X-Org-Id", value: []byte("acme")},
+			{name: "X-Api-Ver", prefix: "v", value: []byte("2")},
+		}, nil)
+	})
+
+	status, body := do(t, client, "GET", fmt.Sprintf("https://%s/x", host), "")
+	if status != http.StatusOK {
+		t.Fatalf("got %d: %s", status, body)
+	}
+	got := decodeEcho(t, body)
+	if v := got.Headers["X-Org-Id"]; len(v) != 1 || v[0] != "acme" {
+		t.Fatalf("X-Org-Id = %v", v)
+	}
+	if v := got.Headers["X-Api-Ver"]; len(v) != 1 || v[0] != "v 2" {
+		t.Fatalf("X-Api-Ver = %v", v)
+	}
+}
+
+func TestSubstitutionsReachTheUpstream(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, nil, nil, []substitution{
+			subOn("__PAT__", "real-token", surfacePath, surfaceQuery, surfaceHeader, surfaceBody),
+		})
+	})
+
+	req, err := http.NewRequest(
+		"POST",
+		fmt.Sprintf("https://%s/repos/__PAT__/x?key=__PAT__", host),
+		strings.NewReader(`{"token":"__PAT__"}`),
+	)
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("X-Key", "Bearer __PAT__")
+
+	resp, err := client.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	payload, _ := io.ReadAll(resp.Body)
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("got %d: %s", resp.StatusCode, payload)
+	}
+
+	got := decodeEcho(t, strings.TrimSpace(string(payload)))
+	if got.Path != "/repos/real-token/x" {
+		t.Fatalf("path = %q", got.Path)
+	}
+	if got.Query != "key=real-token" {
+		t.Fatalf("query = %q", got.Query)
+	}
+	if v := got.Headers["X-Key"]; len(v) != 1 || v[0] != "Bearer real-token" {
+		t.Fatalf("X-Key = %v", v)
+	}
+	if got.Body != `{"token":"real-token"}` {
+		t.Fatalf("body = %q", got.Body)
+	}
+	// The placeholder must be gone from every surface, not merely replaced in the ones we checked.
+	if strings.Contains(string(payload), "__PAT__") {
+		t.Fatalf("a placeholder survived to the upstream: %s", payload)
+	}
+}
+
+func TestABlockedSubstitutedPathNeverEchoesTheSecret(t *testing.T) {
+	// The re-check happens after the real value is in the path, so the refusal must not quote the path:
+	// the 403 body goes back to the agent and the same text goes to the proxy log.
+	secret := "s3cr3t%val"
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
+			subOn("__PAT__", secret, surfacePath),
+		})
+	})
+
+	status, body := do(t, client, "GET", fmt.Sprintf("https://%s/repos/__PAT__", host), "")
+	if status != http.StatusForbidden {
+		t.Fatalf("expected a 403, got %d: %s", status, body)
+	}
+	if strings.Contains(body, "s3cr3t") {
+		t.Fatalf("the 403 body handed the injected secret back to the agent: %q", body)
+	}
+}
+
+func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
+	// The path is authorised before substitution, so a value containing a traversal must not smuggle the
+	// request out of its prefix afterwards.
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
+			subOn("__PAT__", "../admin", surfacePath),
+		})
+	})
+
+	status, body := do(t, client, "GET", fmt.Sprintf("https://%s/repos/__PAT__", host), "")
+	if status != http.StatusForbidden {
+		t.Fatalf("a substitution that escapes the prefix should be refused, got %d: %s", status, body)
+	}
+}
+
+func TestTheLogSaysWhichSurfacesWereSubstituted(t *testing.T) {
+	// The logged path is always the agent's own, placeholder and all, so without this field a
+	// substitution that matched nothing reads exactly like one that fired.
+	type line struct {
+		Path        string   `json:"path"`
+		Decision    string   `json:"decision"`
+		Substituted []string `json:"substituted"`
+	}
+
+	capture := func(t *testing.T, target string) line {
+		t.Helper()
+		client, host := newPolicyFixture(t, func(h string) *resolvedService {
+			return policyService(h, nil, nil, nil, []substitution{
+				subOn("__PAT__", "real-token", surfacePath, surfaceHeader),
+			})
+		})
+
+		var buf bytes.Buffer
+		restore := log.Logger
+		log.Logger = zerolog.New(&buf)
+		defer func() { log.Logger = restore }()
+
+		if status, body := do(t, client, "GET", fmt.Sprintf("https://%s%s", host, target), ""); status != http.StatusOK {
+			t.Fatalf("got %d: %s", status, body)
+		}
+
+		var got line
+		for _, raw := range strings.Split(strings.TrimSpace(buf.String()), "\n") {
+			var candidate line
+			if json.Unmarshal([]byte(raw), &candidate) == nil && candidate.Decision != "" {
+				got = candidate
+			}
+		}
+		if got.Decision == "" {
+			t.Fatalf("no request line logged: %s", buf.String())
+		}
+		return got
+	}
+
+	t.Run("a substitution that fired names its surfaces", func(t *testing.T) {
+		got := capture(t, "/repos/__PAT__/x")
+		if len(got.Substituted) != 1 || got.Substituted[0] != surfacePath {
+			t.Fatalf("substituted = %v, want [path]", got.Substituted)
+		}
+		// The agent's own placeholder, never the value it was swapped for.
+		if got.Path != "/repos/__PAT__/x" {
+			t.Fatalf("path = %q", got.Path)
+		}
+		if strings.Contains(got.Path, "real-token") {
+			t.Fatalf("the log leaked the substituted value: %q", got.Path)
+		}
+	})
+
+	t.Run("a substitution that matched nothing says nothing", func(t *testing.T) {
+		got := capture(t, "/repos/no-placeholder-here")
+		if len(got.Substituted) != 0 {
+			t.Fatalf("substituted = %v, want empty", got.Substituted)
+		}
+	})
+}
diff --git a/packages/agentvault/resolve.go b/packages/agentvault/resolve.go
index 79e8f0ac..a3a38274 100644
--- a/packages/agentvault/resolve.go
+++ b/packages/agentvault/resolve.go
@@ -1,6 +1,7 @@
 package agentvault
 
 import (
+	"strings"
 	"time"
 
 	"github.com/Infisical/infisical-merge/packages/api"
@@ -53,11 +54,15 @@ func (r *infisicalResolver) resolve(sessionToken string) (*resolveResult, error)
 	services := make([]*resolvedService, 0, len(res.Services))
 	for _, wire := range res.Services {
 		services = append(services, &resolvedService{
-			id:               wire.ID,
-			name:             wire.Name,
-			accessBundleName: wire.AccessBundleName,
-			hostPatterns:     parseHostPatterns(wire.HostPattern),
-			credential:       toCredential(wire.Credential),
+			id:                  wire.ID,
+			name:                wire.Name,
+			accessBundleName:    wire.AccessBundleName,
+			hostPatterns:        parseHostPatterns(wire.HostPattern),
+			allowedMethods:      toMethodSet(wire.AllowedMethods),
+			allowedPathPrefixes: toPathPrefixes(wire.AllowedPathPrefixes),
+			credential:          toCredential(wire.Credential),
+			headers:             toHeaders(wire.Headers),
+			substitutions:       toSubstitutions(wire.Substitutions),
 		})
 	}
 
@@ -83,3 +88,69 @@ func toCredential(wire api.AgentVaultCredential) credential {
 		return credential{kind: credentialPassthrough}
 	}
 }
+
+// A nil slice stays a nil map, which allowsMethod reads as "every method". An empty list from the server
+// would be a restriction allowing nothing, so it is kept distinct rather than folded into nil.
+func toMethodSet(methods []string) map[string]bool {
+	if methods == nil {
+		return nil
+	}
+	set := make(map[string]bool, len(methods))
+	for _, method := range methods {
+		set[strings.ToUpper(strings.TrimSpace(method))] = true
+	}
+	return set
+}
+
+// Normalised the same way the backend stores them, so a trailing slash cannot make a prefix unmatchable.
+//
+// Fails closed like toMethodSet: nil means unrestricted, and anything else means restricted, including a
+// list the server sent with nothing usable in it. Dropping to a zero-length slice there would read as
+// unrestricted at every call site, which is the opposite of what a restriction that arrived empty means.
+func toPathPrefixes(prefixes []string) []string {
+	if prefixes == nil {
+		return nil
+	}
+	out := make([]string, 0, len(prefixes))
+	for _, prefix := range prefixes {
+		prefix = strings.TrimSpace(prefix)
+		if prefix == "" {
+			continue
+		}
+		if prefix != "/" {
+			prefix = strings.TrimRight(prefix, "/")
+		}
+		out = append(out, prefix)
+	}
+	if len(out) == 0 {
+		// A prefix no request can match, so a restriction the server sent empty allows nothing.
+		return []string{"\x00"}
+	}
+	return out
+}
+
+func toHeaders(wire []api.AgentVaultHeader) []customHeader {
+	if len(wire) == 0 {
+		return nil
+	}
+	headers := make([]customHeader, 0, len(wire))
+	for _, h := range wire {
+		headers = append(headers, customHeader{name: h.Name, prefix: h.Prefix, value: []byte(h.Value)})
+	}
+	return headers
+}
+
+func toSubstitutions(wire []api.AgentVaultSubstitution) []substitution {
+	if len(wire) == 0 {
+		return nil
+	}
+	subs := make([]substitution, 0, len(wire))
+	for _, s := range wire {
+		surfaces := make(map[string]bool, len(s.Surfaces))
+		for _, surface := range s.Surfaces {
+			surfaces[surface] = true
+		}
+		subs = append(subs, substitution{placeholder: s.Placeholder, surfaces: surfaces, value: []byte(s.Value)})
+	}
+	return subs
+}
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 009fb088..d4982330 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -1,15 +1,27 @@
 package agentvault
 
 import (
+	"bytes"
 	"encoding/base64"
+	"fmt"
+	"io"
 	"net/http"
 	"strings"
+
+	"github.com/rs/zerolog/log"
 )
 
 const (
 	credentialBearer      = "bearer"
 	credentialBasic       = "basic"
 	credentialPassthrough = "passthrough"
+
+	surfacePath   = "path"
+	surfaceQuery  = "query"
+	surfaceHeader = "header"
+	surfaceBody   = "body"
+
+	maxBodyRewriteSize = 10 * 1024 * 1024
 )
 
 // injectCredential overwrites an existing header on the agent's request, silently and deliberately.
@@ -35,6 +47,169 @@ func injectCredential(req *http.Request, cred *credential) bool {
 	}
 }
 
+// injectHeaders writes the service's custom headers, after the credential, so a header whose name collides
+// with the credential's would overwrite it. The backend refuses that pairing on write for exactly this
+// reason; nothing here can detect it, because by this point both are just names.
+func injectHeaders(req *http.Request, headers []customHeader) bool {
+	for _, header := range headers {
+		value := string(header.value)
+		if header.prefix != "" {
+			value = header.prefix + " " + value
+		}
+		req.Header.Set(header.name, value)
+	}
+	return len(headers) > 0
+}
+
+// applySubstitutions swaps each placeholder for its real value across the surfaces the service names.
+// Ported from the agent proxy (packages/agentproxy/rewrite.go), with one deliberate change: a body it
+// cannot rewrite is logged rather than skipped in silence, because the request then goes upstream with the
+// placeholder still in it and the agent only ever sees a third-party 401.
+func applySubstitutions(req *http.Request, serviceName string, subs []substitution) []string {
+	changed := map[string]bool{}
+	for _, sub := range subs {
+		if len(sub.placeholder) == 0 {
+			continue
+		}
+		real := string(sub.value)
+
+		if sub.surfaces[surfacePath] && strings.Contains(req.URL.Path, sub.placeholder) {
+			if v, ok := replaceWithinLimit(req.URL.Path, sub.placeholder, real, maxBodyRewriteSize); ok {
+				req.URL.Path = v
+				// Clearing RawPath makes Go re-encode the path from Path, which can change the byte form of
+				// other escaped segments.
+				req.URL.RawPath = ""
+				changed[surfacePath] = true
+			}
+		}
+
+		if sub.surfaces[surfaceQuery] && strings.Contains(req.URL.RawQuery, sub.placeholder) {
+			if v, ok := replaceWithinLimit(req.URL.RawQuery, sub.placeholder, real, maxBodyRewriteSize); ok {
+				req.URL.RawQuery = v
+				changed[surfaceQuery] = true
+			}
+		}
+
+		if sub.surfaces[surfaceHeader] {
+			for name, values := range req.Header {
+				for i, v := range values {
+					if !strings.Contains(v, sub.placeholder) {
+						continue
+					}
+					if replaced, ok := replaceWithinLimit(v, sub.placeholder, real, maxBodyRewriteSize); ok {
+						req.Header[name][i] = replaced
+						changed[surfaceHeader] = true
+					}
+				}
+			}
+		}
+	}
+
+	if bodySubstitutions(subs) && req.Body != nil {
+		if applyBodySubstitutions(req, serviceName, subs) {
+			changed[surfaceBody] = true
+		}
+	}
+
+	surfaces := make([]string, 0, len(changed))
+	for _, surface := range []string{surfacePath, surfaceQuery, surfaceHeader, surfaceBody} {
+		if changed[surface] {
+			surfaces = append(surfaces, surface)
+		}
+	}
+	return surfaces
+}
+
+func bodySubstitutions(subs []substitution) bool {
+	for _, sub := range subs {
+		if sub.surfaces[surfaceBody] {
+			return true
+		}
+	}
+	return false
+}
+
+// The body is only ever read when some substitution names it, so a service without one keeps streaming
+// exactly as before.
+func applyBodySubstitutions(req *http.Request, serviceName string, subs []substitution) bool {
+	if req.Body == http.NoBody || req.ContentLength == 0 {
+		return false
+	}
+	if enc := req.Header.Get("Content-Encoding"); enc != "" {
+		log.Warn().
+			Str("service", serviceName).
+			Str("contentEncoding", enc).
+			Msg("agent-vault: body substitution skipped on an encoded body; the placeholder is going upstream unchanged")
+		return false
+	}
+
+	body, err := io.ReadAll(io.LimitReader(req.Body, maxBodyRewriteSize+1))
+	if err != nil {
+		// The stream is already part-consumed, so the original length is no longer true. Left alone,
+		// http.Transport refuses the request outright and the agent gets a 502 rather than the unchanged
+		// body this path promises.
+		_ = req.Body.Close()
+		req.Body = io.NopCloser(bytes.NewReader(body))
+		req.ContentLength = int64(len(body))
+		req.Header.Set("Content-Length", fmt.Sprintf("%d", len(body)))
+		log.Warn().Err(err).Str("service", serviceName).
+			Msg("agent-vault: could not read the whole request body for substitution; forwarding what was read, with the placeholder unchanged")
+		return false
+	}
+	if len(body) > maxBodyRewriteSize {
+		req.Body = io.NopCloser(io.MultiReader(bytes.NewReader(body), req.Body))
+		log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
+			Msg("agent-vault: body larger than the substitution limit; the placeholder is going upstream unchanged")
+		return false
+	}
+	_ = req.Body.Close()
+
+	rewritten := body
+	replaced := false
+	for _, sub := range subs {
+		if !sub.surfaces[surfaceBody] || len(sub.placeholder) == 0 {
+			continue
+		}
+		count := bytes.Count(rewritten, []byte(sub.placeholder))
+		if count == 0 {
+			continue
+		}
+		// Forward unchanged when expanding the placeholder would push the body past the cap.
+		if len(rewritten)+count*(len(sub.value)-len(sub.placeholder)) > maxBodyRewriteSize {
+			log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
+				Msg("agent-vault: substituted body would exceed the limit; the placeholder is going upstream unchanged")
+			continue
+		}
+		rewritten = bytes.ReplaceAll(rewritten, []byte(sub.placeholder), sub.value)
+		replaced = true
+	}
+
+	if len(rewritten) == 0 {
+		// A NopCloser over an empty reader reads to net/http as "length unknown", which turns a bodyless
+		// POST into a chunked request. Signing schemes and some gateways reject that.
+		req.Body = http.NoBody
+	} else {
+		req.Body = io.NopCloser(bytes.NewReader(rewritten))
+	}
+	req.ContentLength = int64(len(rewritten))
+	req.Header.Set("Content-Length", fmt.Sprintf("%d", len(rewritten)))
+	return replaced
+}
+
+// replaceWithinLimit substitutes every occurrence of old in s, but only when the expanded result stays
+// within limit bytes; otherwise it returns the input unchanged. This stops a short placeholder mapped to a
+// long secret from ballooning proxy memory, since ReplaceAll allocates by the expansion ratio.
+func replaceWithinLimit(s, old, replacement string, limit int) (string, bool) {
+	count := strings.Count(s, old)
+	if count == 0 {
+		return s, true
+	}
+	if len(s)+count*(len(replacement)-len(old)) > limit {
+		return s, false
+	}
+	return strings.ReplaceAll(s, old, replacement), true
+}
+
 // stripHopByHopHeaders also deletes Upgrade, which is why WebSocket upgrades cannot be forwarded.
 // Callers strip before injecting a credential: a Connection list naming the credential's header would
 // otherwise delete it, which is the same trap Go documents on httputil.ReverseProxy.Director.
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
new file mode 100644
index 00000000..249c9c1e
--- /dev/null
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -0,0 +1,162 @@
+package agentvault
+
+import (
+	"bytes"
+	"io"
+	"net/http"
+	"strings"
+	"testing"
+)
+
+func subOn(placeholder, value string, surfaces ...string) substitution {
+	set := map[string]bool{}
+	for _, surface := range surfaces {
+		set[surface] = true
+	}
+	return substitution{placeholder: placeholder, surfaces: set, value: []byte(value)}
+}
+
+func TestInjectHeaders(t *testing.T) {
+	t.Run("writes name, prefix and value", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
+		injectHeaders(req, []customHeader{
+			{name: "X-Org-Id", prefix: "", value: []byte("acme")},
+			{name: "X-Api-Ver", prefix: "v", value: []byte("2")},
+		})
+		if got := req.Header.Get("X-Org-Id"); got != "acme" {
+			t.Fatalf("X-Org-Id = %q", got)
+		}
+		if got := req.Header.Get("X-Api-Ver"); got != "v 2" {
+			t.Fatalf("X-Api-Ver = %q", got)
+		}
+	})
+
+	t.Run("overwrites whatever the agent sent", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
+		req.Header.Set("X-Org-Id", "spoofed")
+		injectHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
+		if got := req.Header.Get("X-Org-Id"); got != "acme" {
+			t.Fatalf("X-Org-Id = %q", got)
+		}
+	})
+
+	// Same trap injectCredential documents: a Connection list naming the header would delete it.
+	t.Run("a Connection header cannot delete an injected custom header", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
+		req.Header.Set("Connection", "X-Org-Id")
+		stripHopByHopHeaders(req.Header)
+		injectHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
+		if got := req.Header.Get("X-Org-Id"); got != "acme" {
+			t.Fatalf("X-Org-Id = %q", got)
+		}
+	})
+}
+
+func TestApplySubstitutions(t *testing.T) {
+	t.Run("path", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__TOKEN__/x", nil)
+		surfaces := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfacePath)})
+		if req.URL.Path != "/repos/real/x" {
+			t.Fatalf("path = %q", req.URL.Path)
+		}
+		if len(surfaces) != 1 || surfaces[0] != surfacePath {
+			t.Fatalf("surfaces = %v", surfaces)
+		}
+	})
+
+	t.Run("query", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x?key=__TOKEN__", nil)
+		applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceQuery)})
+		if req.URL.RawQuery != "key=real" {
+			t.Fatalf("query = %q", req.URL.RawQuery)
+		}
+	})
+
+	t.Run("header", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
+		req.Header.Set("X-Key", "Bearer __TOKEN__")
+		applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceHeader)})
+		if got := req.Header.Get("X-Key"); got != "Bearer real" {
+			t.Fatalf("X-Key = %q", got)
+		}
+	})
+
+	t.Run("body, with Content-Length corrected", func(t *testing.T) {
+		body := `{"token":"__TOKEN__"}`
+		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
+		applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "realvalue", surfaceBody)})
+		got, _ := io.ReadAll(req.Body)
+		want := `{"token":"realvalue"}`
+		if string(got) != want {
+			t.Fatalf("body = %q", got)
+		}
+		if req.ContentLength != int64(len(want)) {
+			t.Fatalf("ContentLength = %d, want %d", req.ContentLength, len(want))
+		}
+	})
+
+	t.Run("a surface the substitution does not name is left alone", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__TOKEN__", nil)
+		req.Header.Set("X-Key", "__TOKEN__")
+		applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceHeader)})
+		if req.URL.Path != "/repos/__TOKEN__" {
+			t.Fatalf("path should be untouched, got %q", req.URL.Path)
+		}
+		if got := req.Header.Get("X-Key"); got != "real" {
+			t.Fatalf("X-Key = %q", got)
+		}
+	})
+
+	// The placeholder goes upstream unchanged here, which is why the proxy logs it rather than staying quiet.
+	t.Run("an encoded body is forwarded untouched", func(t *testing.T) {
+		body := `{"token":"__TOKEN__"}`
+		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
+		req.Header.Set("Content-Encoding", "gzip")
+		surfaces := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
+		got, _ := io.ReadAll(req.Body)
+		if string(got) != body {
+			t.Fatalf("body should be untouched, got %q", got)
+		}
+		if len(surfaces) != 0 {
+			t.Fatalf("nothing should be reported as changed, got %v", surfaces)
+		}
+	})
+
+	t.Run("a body over the limit is forwarded untouched and still readable", func(t *testing.T) {
+		body := strings.Repeat("a", maxBodyRewriteSize+10) + "__TOKEN__"
+		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
+		applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
+		got, _ := io.ReadAll(req.Body)
+		if !bytes.Equal(got, []byte(body)) {
+			t.Fatalf("an oversize body must be forwarded byte for byte (got %d bytes, want %d)", len(got), len(body))
+		}
+	})
+
+	t.Run("a body with no placeholder in it is untouched", func(t *testing.T) {
+		body := `{"a":"b"}`
+		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
+		surfaces := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
+		got, _ := io.ReadAll(req.Body)
+		if string(got) != body {
+			t.Fatalf("body = %q", got)
+		}
+		if len(surfaces) != 0 {
+			t.Fatalf("surfaces = %v", surfaces)
+		}
+	})
+
+	t.Run("several substitutions apply to one request", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__A__", nil)
+		req.Header.Set("X-Key", "__B__")
+		applySubstitutions(req, "github", []substitution{
+			subOn("__A__", "one", surfacePath),
+			subOn("__B__", "two", surfaceHeader),
+		})
+		if req.URL.Path != "/repos/one" {
+			t.Fatalf("path = %q", req.URL.Path)
+		}
+		if got := req.Header.Get("X-Key"); got != "two" {
+			t.Fatalf("X-Key = %q", got)
+		}
+	})
+}
diff --git a/packages/api/agent_vault.go b/packages/api/agent_vault.go
index 8c9cbadf..13bd5b2a 100644
--- a/packages/api/agent_vault.go
+++ b/packages/api/agent_vault.go
@@ -77,12 +77,29 @@ type AgentVaultCredential struct {
 	Password     string `json:"password,omitempty"`
 }
 
+type AgentVaultHeader struct {
+	Name   string `json:"name"`
+	Prefix string `json:"prefix,omitempty"`
+	Value  string `json:"value"`
+}
+
+type AgentVaultSubstitution struct {
+	Placeholder string   `json:"placeholder"`
+	Surfaces    []string `json:"surfaces"`
+	Value       string   `json:"value"`
+}
+
 type AgentVaultService struct {
-	ID               string               `json:"id"`
-	Name             string               `json:"name"`
-	AccessBundleName string               `json:"accessBundleName"`
-	HostPattern      string               `json:"hostPattern"`
-	Credential       AgentVaultCredential `json:"credential"`
+	ID               string `json:"id"`
+	Name             string `json:"name"`
+	AccessBundleName string `json:"accessBundleName"`
+	HostPattern      string `json:"hostPattern"`
+	// A nil slice means unrestricted, which is what JSON null decodes to.
+	AllowedMethods      []string                 `json:"allowedMethods"`
+	AllowedPathPrefixes []string                 `json:"allowedPathPrefixes"`
+	Credential          AgentVaultCredential     `json:"credential"`
+	Headers             []AgentVaultHeader       `json:"headers"`
+	Substitutions       []AgentVaultSubstitution `json:"substitutions"`
 }
 
 type ResolveAgentVaultSessionResponse struct {

From 4280d8022363ae520d6c20dc4abf6ea5c81f8da9 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Mon, 14 Sep 2026 21:00:46 +0530
Subject: [PATCH 02/25] fix(agent-vault): substitute against the escaped path
 and escape the query value

A path substitution rewrote the decoded Path and cleared RawPath, so Go re-derived
the wire path from it. Its encoder does not re-escape '/' or '+', so a GitLab
project addressed as group%2Fproject arrived at the upstream as two path segments,
addressing a different repository. The swap now runs against EscapedPath and writes
the result back into RawPath, and the replacement is escaped so a secret containing
a slash cannot add a segment of its own.

RawQuery goes on the wire verbatim and had the same bug: a base64 key containing
'+' reached the upstream as a space, and one containing '&' split into a second
parameter. Escaped now.

packages/agentproxy carries the same two lines and is deliberately left alone.
---
 packages/agentvault/rewrite.go                | 29 +++++---
 .../rewrite_transformations_test.go           | 72 +++++++++++++++++++
 2 files changed, 93 insertions(+), 8 deletions(-)

diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index d4982330..b1885064 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 
 	"github.com/rs/zerolog/log"
@@ -73,18 +74,30 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 		}
 		real := string(sub.value)
 
-		if sub.surfaces[surfacePath] && strings.Contains(req.URL.Path, sub.placeholder) {
-			if v, ok := replaceWithinLimit(req.URL.Path, sub.placeholder, real, maxBodyRewriteSize); ok {
-				req.URL.Path = v
-				// Clearing RawPath makes Go re-encode the path from Path, which can change the byte form of
-				// other escaped segments.
-				req.URL.RawPath = ""
-				changed[surfacePath] = true
+		// Swapped in the escaped path and written back escaped, so every other segment keeps the byte form
+		// the agent sent. Rewriting the decoded Path and clearing RawPath (which is what Agent Proxy does)
+		// makes Go re-derive the wire path, and its encoder does not re-escape '/' or '+': a GitLab project
+		// addressed as `group%2Fproject` would arrive as two segments, pointing at a different resource.
+		if sub.surfaces[surfacePath] {
+			escaped := req.URL.EscapedPath()
+			if strings.Contains(escaped, sub.placeholder) {
+				// The replacement is escaped too, or a secret containing '/' or '?' would itself reshape the URL.
+				if v, ok := replaceWithinLimit(escaped, sub.placeholder, url.PathEscape(real), maxBodyRewriteSize); ok {
+					if decoded, err := url.PathUnescape(v); err == nil {
+						// Both halves: Path is what the policy re-check and any later reader see, RawPath is
+						// what goes on the wire. Go uses RawPath only when it agrees with Path.
+						req.URL.Path = decoded
+						req.URL.RawPath = v
+						changed[surfacePath] = true
+					}
+				}
 			}
 		}
 
+		// Escaped, because RawQuery goes on the wire verbatim. A base64 key containing '+' would otherwise
+		// arrive as a space, and one containing '&' would split into a second parameter.
 		if sub.surfaces[surfaceQuery] && strings.Contains(req.URL.RawQuery, sub.placeholder) {
-			if v, ok := replaceWithinLimit(req.URL.RawQuery, sub.placeholder, real, maxBodyRewriteSize); ok {
+			if v, ok := replaceWithinLimit(req.URL.RawQuery, sub.placeholder, url.QueryEscape(real), maxBodyRewriteSize); ok {
 				req.URL.RawQuery = v
 				changed[surfaceQuery] = true
 			}
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 249c9c1e..a63f9e7c 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -4,6 +4,7 @@ import (
 	"bytes"
 	"io"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 )
@@ -160,3 +161,74 @@ func TestApplySubstitutions(t *testing.T) {
 		}
 	})
 }
+
+func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
+	// GitLab addresses a project as group%2Fproject: one name containing a slash, not two segments.
+	// Re-deriving the wire path from the decoded Path would turn it into two and address a different repo.
+	for _, tc := range []struct{ name, target, wantURI string }{
+		{
+			"an encoded slash survives",
+			"https://gitlab.com/api/v4/projects/group%2Fproject/repository/__PAT__",
+			"/api/v4/projects/group%2Fproject/repository/real",
+		},
+		{
+			"an encoded plus survives",
+			"https://api.github.com/repos/a%2Bb/__PAT__",
+			"/repos/a%2Bb/real",
+		},
+		{
+			"an encoded space survives",
+			"https://api.github.com/repos/a%20b/__PAT__",
+			"/repos/a%20b/real",
+		},
+		{
+			"non-ASCII survives",
+			"https://api.github.com/repos/caf%C3%A9/__PAT__",
+			"/repos/caf%C3%A9/real",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req, err := http.NewRequest("GET", tc.target, nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			applySubstitutions(req, "gitlab", []substitution{subOn("__PAT__", "real", surfacePath)})
+			if got := req.URL.RequestURI(); got != tc.wantURI {
+				t.Fatalf("wire path = %q, want %q", got, tc.wantURI)
+			}
+		})
+	}
+
+	t.Run("a secret containing a slash cannot add a segment", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__PAT__", nil)
+		applySubstitutions(req, "github", []substitution{subOn("__PAT__", "a/b", surfacePath)})
+		if got := req.URL.RequestURI(); got != "/repos/a%2Fb" {
+			t.Fatalf("wire path = %q, want the slash escaped", got)
+		}
+	})
+}
+
+func TestAQuerySubstitutionEscapesTheValue(t *testing.T) {
+	// secretValueSchema allows '+', '&' and spaces, and RawQuery goes on the wire verbatim.
+	for _, tc := range []struct{ name, secret, wantKey string }{
+		{"a base64 key with a plus", "aB+cD/eF==", "aB+cD/eF=="},
+		{"a value with a space", "has space", "has space"},
+		{"a value with an ampersand", "a&page=99", "a&page=99"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req, _ := http.NewRequest("GET", "https://api.example.com/x?key=__PAT__&page=2", nil)
+			applySubstitutions(req, "svc", []substitution{subOn("__PAT__", tc.secret, surfaceQuery)})
+
+			parsed, err := url.ParseQuery(req.URL.RawQuery)
+			if err != nil {
+				t.Fatalf("the query no longer parses: %v", err)
+			}
+			if got := parsed.Get("key"); got != tc.wantKey {
+				t.Fatalf("upstream reads key=%q, want %q", got, tc.wantKey)
+			}
+			if got := parsed.Get("page"); got != "2" {
+				t.Fatalf("the substitution disturbed another parameter: page=%q", got)
+			}
+		})
+	}
+}

From 310a41e3cbeb5aad84931e30ba1ed402379db7ad Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 07:47:14 +0530
Subject: [PATCH 03/25] fix(agent-vault): match encoded placeholders, accept
 our own path escapes, give the credential precedence

A placeholder carrying a character Go percent-escapes in a path never matched.
EscapedPath re-encodes the whole path whenever what the agent sent is not
already valid encoding, so a `{{TOKEN}}` on the wire reads as `%7B%7BTOKEN%7D%7D`
and the swap looked only for the form the author typed. The same placeholder
worked in the query string and silently did nothing in the path, and the agent
saw a third-party 404 with no warning. The swap now looks for the encoded form
too, produced by the same encoder EscapedPath falls back to.

The post-substitution path re-check refused the escaping applySubstitutions
itself writes. The replacement is PathEscaped precisely so a secret containing a
slash cannot add a segment, and that `%2F` is what isAmbiguousPath reads as a
separator: a GitLab project addressed as group%2Fproject answered 403 against a
prefix that plainly covered it, and the refusal blamed the path rule. The
re-check keeps the byte-exact prefix comparison, which is what holds the
substituted span after the prefix, and refuses only traversal, judged on the
decoded path because an upstream that decodes %2F before routing is the reader
`..%2F..%2Fadmin` is written for.

Custom headers are written before the credential rather than after, reversing
the order the previous commit set. One naming the credential's own header now
loses to it instead of replacing the real token with a custom value the agent
cannot explain. The backend refuses that pairing on write, so this is the floor
under that check rather than a replacement for it: a service saved before the
check existed, or a write that raced it, can no longer cost an agent its
credential. Pass-through injects nothing, so a custom Authorization header on
one still lands.
---
 packages/agentvault/policy.go                 | 23 ++++++
 packages/agentvault/proxy.go                  | 13 ++--
 packages/agentvault/proxy_policy_test.go      | 77 ++++++++++++++++++-
 packages/agentvault/rewrite.go                | 25 ++++--
 .../rewrite_transformations_test.go           | 12 +++
 5 files changed, 139 insertions(+), 11 deletions(-)

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index 1fc435c2..ad6c7df1 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -65,6 +65,29 @@ func pathAllowed(escaped string, prefixes []string) bool {
 	if isAmbiguousPath(escaped) {
 		return false
 	}
+	return matchesPrefix(escaped, prefixes)
+}
+
+// pathAllowedAfterSubstitution judges a path the proxy itself part-wrote, so it cannot use the rule above.
+// applySubstitutions percent-escapes the value precisely so a secret containing '/' cannot add a segment,
+// and that escape is the '%2F' isAmbiguousPath refuses: judged by pathAllowed, a GitLab project addressed
+// as `group%2Fproject` would 403 against a prefix that plainly covers it.
+//
+// The prefix comparison is unchanged and still byte-exact, which is what keeps the substituted span after
+// the prefix: a placeholder sitting inside the prefix region rewrites those bytes and fails the comparison.
+// That leaves traversal as the only way out of an allowed prefix, so it is the only thing still refused,
+// and it is judged on the decoded path because an upstream that decodes '%2F' before routing is exactly
+// the reader `..%2F..%2Fadmin` is written for.
+func pathAllowedAfterSubstitution(escaped, decoded string, prefixes []string) bool {
+	for _, segment := range strings.Split(decoded, "/") {
+		if segment == "." || segment == ".." {
+			return false
+		}
+	}
+	return matchesPrefix(escaped, prefixes)
+}
+
+func matchesPrefix(escaped string, prefixes []string) bool {
 	for _, prefix := range prefixes {
 		if prefix == "/" {
 			return true
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 0d5682ac..5c387ab9 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -495,11 +495,14 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 				Msg("agent-vault: refusing to attach a credential over plaintext http")
 			matched = nil
 		} else {
-			// Substitutions run before the credential so an injected real value can never itself be rewritten,
-			// and custom headers last so they are not clobbered by it.
+			// Substitutions run first so an injected real value can never itself be rewritten. The credential
+			// goes last and wins: a custom header naming the credential's own header would otherwise replace
+			// the real token with the custom value, and the agent would get a 401 nothing in the service
+			// explains. The backend refuses that pairing on write, so this is the floor under it rather than
+			// the only guard.
 			outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
-			outcome.brokered = injectCredential(req, &matched.credential)
-			if injectHeaders(req, matched.headers) {
+			outcome.brokered = injectHeaders(req, matched.headers)
+			if injectCredential(req, &matched.credential) {
 				outcome.brokered = true
 			}
 			if len(outcome.substituted) > 0 {
@@ -509,7 +512,7 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 			// A path-surface substitution rewrites the path after the check above, so a restricted service
 			// re-checks what actually goes on the wire.
 			if len(matched.allowedPathPrefixes) > 0 && containsSurface(outcome.substituted, surfacePath) {
-				if !pathAllowed(requestPath(req), matched.allowedPathPrefixes) {
+				if !pathAllowedAfterSubstitution(requestPath(req), req.URL.Path, matched.allowedPathPrefixes) {
 					// The path now carries the real credential, so it must not reach the body or the log.
 					// Every other refusal in this file is fixed text for the same reason.
 					return nil, matched, outcome, fmt.Errorf(
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index 62ee2066..90d2fed9 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -246,7 +246,7 @@ func TestSubstitutionsReachTheUpstream(t *testing.T) {
 func TestABlockedSubstitutedPathNeverEchoesTheSecret(t *testing.T) {
 	// The re-check happens after the real value is in the path, so the refusal must not quote the path:
 	// the 403 body goes back to the agent and the same text goes to the proxy log.
-	secret := "s3cr3t%val"
+	secret := "../s3cr3tadmin"
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
 			subOn("__PAT__", secret, surfacePath),
@@ -277,6 +277,81 @@ func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
 	}
 }
 
+func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
+	// applySubstitutions escapes the value so it cannot add a segment, and that escape must not then read
+	// as the ambiguity it was written to prevent: a GitLab project is addressed as `group%2Fproject`.
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, []string{"/api/v4/projects"}, nil, []substitution{
+			subOn("__PROJ__", "mygroup/myproject", surfacePath),
+		})
+	})
+
+	status, body := do(t, client, "GET", fmt.Sprintf("https://%s/api/v4/projects/__PROJ__/pipelines", host), "")
+	if status != http.StatusOK {
+		t.Fatalf("expected a 200, got %d: %s", status, body)
+	}
+	if got := decodeEcho(t, strings.TrimSpace(body)); got.Path != "/api/v4/projects/mygroup%2Fmyproject/pipelines" {
+		t.Fatalf("upstream path = %q", got.Path)
+	}
+}
+
+// The backend refuses this pairing on write, so reaching it means a service saved before that check or a
+// write that raced it. Either way the real token has to survive.
+func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
+	cases := []struct {
+		name       string
+		cred       credential
+		headers    []customHeader
+		wantHeader string
+		want       string
+	}{
+		{
+			name:       "the default Authorization, collided case-insensitively",
+			cred:       credential{kind: credentialBearer, headerPrefix: "Bearer", value: []byte("real-token")},
+			headers:    []customHeader{{name: "authorization", value: []byte("spoofed")}},
+			wantHeader: "Authorization",
+			want:       "Bearer real-token",
+		},
+		{
+			name:       "a credential on its own header name",
+			cred:       credential{kind: credentialBearer, headerName: "X-Org-Id", value: []byte("real-token")},
+			headers:    []customHeader{{name: "X-Org-Id", value: []byte("spoofed")}},
+			wantHeader: "X-Org-Id",
+			want:       "real-token",
+		},
+		{
+			// Pass-through injects no credential, so a custom Authorization header is the whole point.
+			name:       "pass-through leaves the custom header alone",
+			cred:       credential{kind: credentialPassthrough},
+			headers:    []customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("custom")}},
+			wantHeader: "Authorization",
+			want:       "Bearer custom",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			client, host := newPolicyFixture(t, func(h string) *resolvedService {
+				svc := policyService(h, nil, nil, tc.headers, nil)
+				svc.credential = tc.cred
+				return svc
+			})
+
+			status, body := do(t, client, "GET", fmt.Sprintf("https://%s/x", host), "")
+			if status != http.StatusOK {
+				t.Fatalf("got %d: %s", status, body)
+			}
+			got := decodeEcho(t, strings.TrimSpace(body))
+			if v := got.Headers[tc.wantHeader]; len(v) != 1 || v[0] != tc.want {
+				t.Fatalf("%s = %v, want %q", tc.wantHeader, v, tc.want)
+			}
+			if strings.Contains(body, "spoofed") {
+				t.Fatalf("the custom header replaced the credential: %s", body)
+			}
+		})
+	}
+}
+
 func TestTheLogSaysWhichSurfacesWereSubstituted(t *testing.T) {
 	// The logged path is always the agent's own, placeholder and all, so without this field a
 	// substitution that matched nothing reads exactly like one that fired.
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index b1885064..c628ea94 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -48,9 +48,9 @@ func injectCredential(req *http.Request, cred *credential) bool {
 	}
 }
 
-// injectHeaders writes the service's custom headers, after the credential, so a header whose name collides
-// with the credential's would overwrite it. The backend refuses that pairing on write for exactly this
-// reason; nothing here can detect it, because by this point both are just names.
+// injectHeaders writes the service's custom headers, before the credential, so one whose name collides with
+// the credential's loses to it rather than replacing the real token. A pass-through service injects no
+// credential at all, which is why setting Authorization as a custom header on one still works.
 func injectHeaders(req *http.Request, headers []customHeader) bool {
 	for _, header := range headers {
 		value := string(header.value)
@@ -80,9 +80,17 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 		// addressed as `group%2Fproject` would arrive as two segments, pointing at a different resource.
 		if sub.surfaces[surfacePath] {
 			escaped := req.URL.EscapedPath()
-			if strings.Contains(escaped, sub.placeholder) {
+			// EscapedPath re-encodes the whole path whenever what the agent sent is not already valid
+			// encoding, so a `{{TOKEN}}` on the wire reads here as `%7B%7BTOKEN%7D%7D`. Looking only for the
+			// form the author typed would leave those placeholders on the wire, and the agent would see
+			// nothing but a third-party 404.
+			needle := sub.placeholder
+			if !strings.Contains(escaped, needle) {
+				needle = escapedPathForm(sub.placeholder)
+			}
+			if strings.Contains(escaped, needle) {
 				// The replacement is escaped too, or a secret containing '/' or '?' would itself reshape the URL.
-				if v, ok := replaceWithinLimit(escaped, sub.placeholder, url.PathEscape(real), maxBodyRewriteSize); ok {
+				if v, ok := replaceWithinLimit(escaped, needle, url.PathEscape(real), maxBodyRewriteSize); ok {
 					if decoded, err := url.PathUnescape(v); err == nil {
 						// Both halves: Path is what the policy re-check and any later reader see, RawPath is
 						// what goes on the wire. Go uses RawPath only when it agrees with Path.
@@ -133,6 +141,13 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 	return surfaces
 }
 
+// escapedPathForm is the placeholder as EscapedPath would render it: the same encoder, reached the same way.
+// The leading '/' keeps url.URL's `Path == "*"` special case out of it, and the path encoder leaves a slash
+// alone, so trimming it back off is exact.
+func escapedPathForm(placeholder string) string {
+	return strings.TrimPrefix((&url.URL{Path: "/" + placeholder}).EscapedPath(), "/")
+}
+
 func bodySubstitutions(subs []substitution) bool {
 	for _, sub := range subs {
 		if sub.surfaces[surfaceBody] {
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index a63f9e7c..7ac7f7e4 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -65,6 +65,18 @@ func TestApplySubstitutions(t *testing.T) {
 		}
 	})
 
+	// Go escapes '{' in a path, so EscapedPath carries the placeholder in a form the author never typed.
+	t.Run("path, placeholder Go re-encodes", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://gitlab.com/api/v4/projects/{{PROJECT}}/pipelines", nil)
+		surfaces := applySubstitutions(req, "gitlab", []substitution{subOn("{{PROJECT}}", "group/project", surfacePath)})
+		if len(surfaces) != 1 || surfaces[0] != surfacePath {
+			t.Fatalf("surfaces = %v", surfaces)
+		}
+		if got := req.URL.RequestURI(); got != "/api/v4/projects/group%2Fproject/pipelines" {
+			t.Fatalf("wire path = %q", got)
+		}
+	})
+
 	t.Run("query", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x?key=__TOKEN__", nil)
 		applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceQuery)})

From a1dec16792f6312624b84391c2b745c4a0dc298d Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 08:59:24 +0530
Subject: [PATCH 04/25] refactor(agent-vault): call them custom headers on the
 proxy side too

Follows the backend rename. The resolve payload field is `customHeaders`, so an
older proxy reading `headers` finds nothing and a service's custom headers stop
being attached until the binary is updated; the product is in preview and the
field has no other consumer.

  AgentVaultHeader / json:"headers" -> AgentVaultCustomHeader / json:"customHeaders"
  resolvedService.headers           -> resolvedService.customHeaders
  injectHeaders                     -> injectCustomHeaders

stripHopByHopHeaders, req.Header and every other genuine HTTP-header name is left
alone: this is only the name of the service's own configured headers.
---
 packages/agentvault/cache.go                  |  2 +-
 packages/agentvault/proxy.go                  |  2 +-
 packages/agentvault/proxy_policy_test.go      | 46 +++++++++----------
 packages/agentvault/resolve.go                | 10 ++--
 packages/agentvault/rewrite.go                |  8 ++--
 .../rewrite_transformations_test.go           |  8 ++--
 packages/api/agent_vault.go                   |  4 +-
 7 files changed, 40 insertions(+), 40 deletions(-)

diff --git a/packages/agentvault/cache.go b/packages/agentvault/cache.go
index a94770f1..09c401b8 100644
--- a/packages/agentvault/cache.go
+++ b/packages/agentvault/cache.go
@@ -60,7 +60,7 @@ type resolvedService struct {
 	allowedMethods      map[string]bool
 	allowedPathPrefixes []string
 	credential          credential
-	headers             []customHeader
+	customHeaders       []customHeader
 	substitutions       []substitution
 }
 
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 5c387ab9..cd27dcdc 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -501,7 +501,7 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 			// explains. The backend refuses that pairing on write, so this is the floor under it rather than
 			// the only guard.
 			outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
-			outcome.brokered = injectHeaders(req, matched.headers)
+			outcome.brokered = injectCustomHeaders(req, matched.customHeaders)
 			if injectCredential(req, &matched.credential) {
 				outcome.brokered = true
 			}
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index 90d2fed9..0fe81dfe 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -92,7 +92,7 @@ func newPolicyFixture(t *testing.T, build func(host string) *resolvedService) (*
 	return client, host
 }
 
-func policyService(host string, methods, prefixes []string, headers []customHeader, subs []substitution) *resolvedService {
+func policyService(host string, methods, prefixes []string, customHeaders []customHeader, subs []substitution) *resolvedService {
 	return &resolvedService{
 		name:                "github",
 		accessBundleName:    "bundle",
@@ -100,7 +100,7 @@ func policyService(host string, methods, prefixes []string, headers []customHead
 		allowedMethods:      toMethodSet(methods),
 		allowedPathPrefixes: toPathPrefixes(prefixes),
 		credential:          credential{kind: credentialPassthrough},
-		headers:             headers,
+		customHeaders:       customHeaders,
 		substitutions:       subs,
 	}
 }
@@ -299,40 +299,40 @@ func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
 // write that raced it. Either way the real token has to survive.
 func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
 	cases := []struct {
-		name       string
-		cred       credential
-		headers    []customHeader
-		wantHeader string
-		want       string
+		name          string
+		cred          credential
+		customHeaders []customHeader
+		wantHeader    string
+		want          string
 	}{
 		{
-			name:       "the default Authorization, collided case-insensitively",
-			cred:       credential{kind: credentialBearer, headerPrefix: "Bearer", value: []byte("real-token")},
-			headers:    []customHeader{{name: "authorization", value: []byte("spoofed")}},
-			wantHeader: "Authorization",
-			want:       "Bearer real-token",
+			name:          "the default Authorization, collided case-insensitively",
+			cred:          credential{kind: credentialBearer, headerPrefix: "Bearer", value: []byte("real-token")},
+			customHeaders: []customHeader{{name: "authorization", value: []byte("spoofed")}},
+			wantHeader:    "Authorization",
+			want:          "Bearer real-token",
 		},
 		{
-			name:       "a credential on its own header name",
-			cred:       credential{kind: credentialBearer, headerName: "X-Org-Id", value: []byte("real-token")},
-			headers:    []customHeader{{name: "X-Org-Id", value: []byte("spoofed")}},
-			wantHeader: "X-Org-Id",
-			want:       "real-token",
+			name:          "a credential on its own header name",
+			cred:          credential{kind: credentialBearer, headerName: "X-Org-Id", value: []byte("real-token")},
+			customHeaders: []customHeader{{name: "X-Org-Id", value: []byte("spoofed")}},
+			wantHeader:    "X-Org-Id",
+			want:          "real-token",
 		},
 		{
 			// Pass-through injects no credential, so a custom Authorization header is the whole point.
-			name:       "pass-through leaves the custom header alone",
-			cred:       credential{kind: credentialPassthrough},
-			headers:    []customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("custom")}},
-			wantHeader: "Authorization",
-			want:       "Bearer custom",
+			name:          "pass-through leaves the custom header alone",
+			cred:          credential{kind: credentialPassthrough},
+			customHeaders: []customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("custom")}},
+			wantHeader:    "Authorization",
+			want:          "Bearer custom",
 		},
 	}
 
 	for _, tc := range cases {
 		t.Run(tc.name, func(t *testing.T) {
 			client, host := newPolicyFixture(t, func(h string) *resolvedService {
-				svc := policyService(h, nil, nil, tc.headers, nil)
+				svc := policyService(h, nil, nil, tc.customHeaders, nil)
 				svc.credential = tc.cred
 				return svc
 			})
diff --git a/packages/agentvault/resolve.go b/packages/agentvault/resolve.go
index a3a38274..20d12c8b 100644
--- a/packages/agentvault/resolve.go
+++ b/packages/agentvault/resolve.go
@@ -61,7 +61,7 @@ func (r *infisicalResolver) resolve(sessionToken string) (*resolveResult, error)
 			allowedMethods:      toMethodSet(wire.AllowedMethods),
 			allowedPathPrefixes: toPathPrefixes(wire.AllowedPathPrefixes),
 			credential:          toCredential(wire.Credential),
-			headers:             toHeaders(wire.Headers),
+			customHeaders:       toCustomHeaders(wire.CustomHeaders),
 			substitutions:       toSubstitutions(wire.Substitutions),
 		})
 	}
@@ -129,15 +129,15 @@ func toPathPrefixes(prefixes []string) []string {
 	return out
 }
 
-func toHeaders(wire []api.AgentVaultHeader) []customHeader {
+func toCustomHeaders(wire []api.AgentVaultCustomHeader) []customHeader {
 	if len(wire) == 0 {
 		return nil
 	}
-	headers := make([]customHeader, 0, len(wire))
+	customHeaders := make([]customHeader, 0, len(wire))
 	for _, h := range wire {
-		headers = append(headers, customHeader{name: h.Name, prefix: h.Prefix, value: []byte(h.Value)})
+		customHeaders = append(customHeaders, customHeader{name: h.Name, prefix: h.Prefix, value: []byte(h.Value)})
 	}
-	return headers
+	return customHeaders
 }
 
 func toSubstitutions(wire []api.AgentVaultSubstitution) []substitution {
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index c628ea94..29d6a15f 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -48,18 +48,18 @@ func injectCredential(req *http.Request, cred *credential) bool {
 	}
 }
 
-// injectHeaders writes the service's custom headers, before the credential, so one whose name collides with
+// injectCustomHeaders writes the service's custom headers, before the credential, so one whose name collides with
 // the credential's loses to it rather than replacing the real token. A pass-through service injects no
 // credential at all, which is why setting Authorization as a custom header on one still works.
-func injectHeaders(req *http.Request, headers []customHeader) bool {
-	for _, header := range headers {
+func injectCustomHeaders(req *http.Request, customHeaders []customHeader) bool {
+	for _, header := range customHeaders {
 		value := string(header.value)
 		if header.prefix != "" {
 			value = header.prefix + " " + value
 		}
 		req.Header.Set(header.name, value)
 	}
-	return len(headers) > 0
+	return len(customHeaders) > 0
 }
 
 // applySubstitutions swaps each placeholder for its real value across the surfaces the service names.
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 7ac7f7e4..82590947 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -17,10 +17,10 @@ func subOn(placeholder, value string, surfaces ...string) substitution {
 	return substitution{placeholder: placeholder, surfaces: set, value: []byte(value)}
 }
 
-func TestInjectHeaders(t *testing.T) {
+func TestInjectCustomHeaders(t *testing.T) {
 	t.Run("writes name, prefix and value", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
-		injectHeaders(req, []customHeader{
+		injectCustomHeaders(req, []customHeader{
 			{name: "X-Org-Id", prefix: "", value: []byte("acme")},
 			{name: "X-Api-Ver", prefix: "v", value: []byte("2")},
 		})
@@ -35,7 +35,7 @@ func TestInjectHeaders(t *testing.T) {
 	t.Run("overwrites whatever the agent sent", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
 		req.Header.Set("X-Org-Id", "spoofed")
-		injectHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
+		injectCustomHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
 		if got := req.Header.Get("X-Org-Id"); got != "acme" {
 			t.Fatalf("X-Org-Id = %q", got)
 		}
@@ -46,7 +46,7 @@ func TestInjectHeaders(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
 		req.Header.Set("Connection", "X-Org-Id")
 		stripHopByHopHeaders(req.Header)
-		injectHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
+		injectCustomHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
 		if got := req.Header.Get("X-Org-Id"); got != "acme" {
 			t.Fatalf("X-Org-Id = %q", got)
 		}
diff --git a/packages/api/agent_vault.go b/packages/api/agent_vault.go
index 13bd5b2a..9dbb4708 100644
--- a/packages/api/agent_vault.go
+++ b/packages/api/agent_vault.go
@@ -77,7 +77,7 @@ type AgentVaultCredential struct {
 	Password     string `json:"password,omitempty"`
 }
 
-type AgentVaultHeader struct {
+type AgentVaultCustomHeader struct {
 	Name   string `json:"name"`
 	Prefix string `json:"prefix,omitempty"`
 	Value  string `json:"value"`
@@ -98,7 +98,7 @@ type AgentVaultService struct {
 	AllowedMethods      []string                 `json:"allowedMethods"`
 	AllowedPathPrefixes []string                 `json:"allowedPathPrefixes"`
 	Credential          AgentVaultCredential     `json:"credential"`
-	Headers             []AgentVaultHeader       `json:"headers"`
+	CustomHeaders       []AgentVaultCustomHeader `json:"customHeaders"`
 	Substitutions       []AgentVaultSubstitution `json:"substitutions"`
 }
 

From 714b9bf8aaa2879d63c80949654254592f99abd7 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 10:07:32 +0530
Subject: [PATCH 05/25] fix(agent-vault): refuse to forward a truncated body
 rather than relabel it

A body read that failed partway was forwarded with ContentLength corrected to what
had been read, which hands the upstream a well-formed shorter request it cannot
tell from a complete one. Verified against a real transport: a body breaking at 300
of 1007 bytes arrived as a clean 300 byte request and the upstream answered 200, so
a broken upload became a partial write nobody can take back.

The declared length is left alone now, so the two disagree and http.Transport
refuses the request outright. The upstream receives nothing and the agent gets a
502 it may not even be alive to see, which is the better end of that trade. The
comment defending the old behaviour weighed the 502 against "the unchanged body
this path promises", but the path cannot promise an intact body once the stream
has broken.

Only reachable on a service carrying a body substitution; everything else never
reads the body at all.
---
 packages/agentvault/rewrite.go                | 14 +++---
 .../rewrite_transformations_test.go           | 43 +++++++++++++++++++
 2 files changed, 50 insertions(+), 7 deletions(-)

diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 29d6a15f..165e2e21 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -173,15 +173,15 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 
 	body, err := io.ReadAll(io.LimitReader(req.Body, maxBodyRewriteSize+1))
 	if err != nil {
-		// The stream is already part-consumed, so the original length is no longer true. Left alone,
-		// http.Transport refuses the request outright and the agent gets a 502 rather than the unchanged
-		// body this path promises.
+		// Deliberately not correcting ContentLength to what was actually read. The declared length and the
+		// bytes now disagree, so http.Transport refuses the request and nothing reaches the upstream. Making
+		// them agree would hand the upstream a well-formed shorter request it cannot tell from a complete
+		// one, turning a broken upload into a partial write nobody can take back. A 502 the agent may not
+		// even be alive to see is the better end of that trade.
 		_ = req.Body.Close()
 		req.Body = io.NopCloser(bytes.NewReader(body))
-		req.ContentLength = int64(len(body))
-		req.Header.Set("Content-Length", fmt.Sprintf("%d", len(body)))
-		log.Warn().Err(err).Str("service", serviceName).
-			Msg("agent-vault: could not read the whole request body for substitution; forwarding what was read, with the placeholder unchanged")
+		log.Warn().Err(err).Str("service", serviceName).Int("bytesRead", len(body)).
+			Msg("agent-vault: could not read the whole request body for substitution; refusing to forward a truncated one")
 		return false
 	}
 	if len(body) > maxBodyRewriteSize {
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 82590947..242fc516 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -2,6 +2,8 @@ package agentvault
 
 import (
 	"bytes"
+	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/url"
@@ -53,6 +55,47 @@ func TestInjectCustomHeaders(t *testing.T) {
 	})
 }
 
+// Dies mid-upload, the way a client that goes away does: some bytes, then an error.
+type halfBody struct {
+	data  []byte
+	n     int
+	limit int
+}
+
+func (b *halfBody) Read(p []byte) (int, error) {
+	if b.n >= b.limit {
+		return 0, errors.New("unexpected EOF")
+	}
+	c := copy(p, b.data[b.n:b.limit])
+	b.n += c
+	return c, nil
+}
+
+func (b *halfBody) Close() error { return nil }
+
+// Correcting the length here would hand the upstream a well-formed shorter request it cannot tell from a
+// complete one. Leaving the two disagreeing is what makes http.Transport refuse to send anything.
+func TestABrokenUploadIsNotForwardedTruncated(t *testing.T) {
+	full := strings.Repeat("A", 500) + "__PAT__" + strings.Repeat("B", 500)
+	req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
+	req.Body = &halfBody{data: []byte(full), limit: 300}
+	req.ContentLength = int64(len(full))
+	req.Header.Set("Content-Length", fmt.Sprintf("%d", len(full)))
+
+	applyBodySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)})
+
+	if req.ContentLength != int64(len(full)) {
+		t.Fatalf("ContentLength = %d, want the declared %d so the transport refuses", req.ContentLength, len(full))
+	}
+	if got := req.Header.Get("Content-Length"); got != fmt.Sprintf("%d", len(full)) {
+		t.Fatalf("Content-Length header = %q, want the declared length", got)
+	}
+	sent, _ := io.ReadAll(req.Body)
+	if len(sent) >= len(full) {
+		t.Fatalf("body = %d bytes, expected only the part that was read", len(sent))
+	}
+}
+
 func TestApplySubstitutions(t *testing.T) {
 	t.Run("path", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__TOKEN__/x", nil)

From e39af566bca9da3194d0378030ad3fcce7e632a7 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 10:27:10 +0530
Subject: [PATCH 06/25] fix(agent-vault): close two gaps review found in the
 substitution path

The post-substitution path re-check refused a bare `..` segment and nothing else,
which is narrower than the pre-check it replaced for a restricted service. `..`
is not the only way up: Tomcat and Jetty strip `;params` per segment and IIS reads
'\' as a separator, so with prefix /repos an agent sending `/repos/..__P__/admin`
walked out to /admin on those upstreams whenever the substituted value began with
'\' or ';'. Verified both, and that `..%2F` was already caught. ';' and '\' are
refused on the decoded path now. The escape-shape checks isAmbiguousPath runs are
still deliberately not repeated here, since the substituted span is percent-escaped
on the way out and running them on the decoded form would refuse a value merely
containing a '%'.

The body path measured after reading, so a request over the 10 MiB cap cost the cap
in memory before being refused, and maxConcurrentConns is 512. A declared length
over the cap now skips the read entirely. The check after reading still stands on
its own: a chunked request declares -1, and a declared length is a claim rather
than a fact.
---
 packages/agentvault/policy.go                 | 16 +++++++++++---
 packages/agentvault/proxy_policy_test.go      | 19 ++++++++++++++++
 packages/agentvault/rewrite.go                |  9 ++++++++
 .../rewrite_transformations_test.go           | 22 +++++++++++++++++++
 4 files changed, 63 insertions(+), 3 deletions(-)

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index ad6c7df1..b797b8b0 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -75,10 +75,20 @@ func pathAllowed(escaped string, prefixes []string) bool {
 //
 // The prefix comparison is unchanged and still byte-exact, which is what keeps the substituted span after
 // the prefix: a placeholder sitting inside the prefix region rewrites those bytes and fails the comparison.
-// That leaves traversal as the only way out of an allowed prefix, so it is the only thing still refused,
-// and it is judged on the decoded path because an upstream that decodes '%2F' before routing is exactly
-// the reader `..%2F..%2Fadmin` is written for.
+// That leaves traversal as the only way out of an allowed prefix, so traversal is what is still refused,
+// judged on the decoded path because an upstream that decodes '%2F' before routing is exactly the reader
+// `..%2F..%2Fadmin` is written for.
+//
+// Traversal is not only a bare `..` segment. ';' and '\' are refused here for the same reason
+// isAmbiguousPath refuses them: Tomcat and Jetty strip `;params` per segment and IIS reads '\' as a
+// separator, so `..;x` and `..\admin` both walk up on some upstream while reading as an ordinary segment
+// to a splitter. The escape-shape checks isAmbiguousPath also runs are deliberately not repeated, since
+// the substituted span is percent-escaped by applySubstitutions and it is the decoded meaning that matters
+// here; running them on the decoded form would refuse a secret merely containing a '%'.
 func pathAllowedAfterSubstitution(escaped, decoded string, prefixes []string) bool {
+	if strings.ContainsAny(decoded, ";\\") {
+		return false
+	}
 	for _, segment := range strings.Split(decoded, "/") {
 		if segment == "." || segment == ".." {
 			return false
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index 0fe81dfe..a02d9bd9 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -277,6 +277,25 @@ func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
 	}
 }
 
+// `..` alone is not the only way out of a prefix: some upstreams read ';' and '\\' as separators, so a
+// value glued onto an agent-supplied `..` walks up there while reading as an ordinary segment to a splitter.
+func TestASubstitutedValueCannotWalkOutOfItsPrefix(t *testing.T) {
+	for _, secret := range []string{`\admin`, `;x`, `/admin`} {
+		t.Run(secret, func(t *testing.T) {
+			client, host := newPolicyFixture(t, func(h string) *resolvedService {
+				return policyService(h, nil, []string{"/repos"}, nil, []substitution{
+					subOn("__P__", secret, surfacePath),
+				})
+			})
+
+			status, body := do(t, client, "GET", fmt.Sprintf("https://%s/repos/..__P__/admin", host), "")
+			if status != http.StatusForbidden {
+				t.Fatalf("expected a 403, got %d: %s", status, body)
+			}
+		})
+	}
+}
+
 func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
 	// applySubstitutions escapes the value so it cannot add a segment, and that escape must not then read
 	// as the ambiguity it was written to prevent: a GitLab project is addressed as `group%2Fproject`.
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 165e2e21..249bcb34 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -170,6 +170,15 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 			Msg("agent-vault: body substitution skipped on an encoded body; the placeholder is going upstream unchanged")
 		return false
 	}
+	// Judged before reading, so a body that already says it is too big costs no memory at all. The check
+	// below still has to stand on its own: a chunked request declares -1, and a declared length is the
+	// client's claim rather than a fact.
+	if req.ContentLength > maxBodyRewriteSize {
+		log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
+			Int64("declaredBytes", req.ContentLength).
+			Msg("agent-vault: body larger than the substitution limit; the placeholder is going upstream unchanged")
+		return false
+	}
 
 	body, err := io.ReadAll(io.LimitReader(req.Body, maxBodyRewriteSize+1))
 	if err != nil {
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 242fc516..797569fc 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -55,6 +55,28 @@ func TestInjectCustomHeaders(t *testing.T) {
 	})
 }
 
+// Fails the test if anything reads it.
+type unreadableBody struct{ t *testing.T }
+
+func (b *unreadableBody) Read([]byte) (int, error) {
+	b.t.Fatal("body was read even though the declared length is over the limit")
+	return 0, nil
+}
+
+func (b *unreadableBody) Close() error { return nil }
+
+// Reading first and measuring afterwards means every oversize request costs the cap in memory before it is
+// refused, which with the connection limit is several gigabytes an agent can make the proxy hold.
+func TestAnOversizedDeclaredBodyIsNeverRead(t *testing.T) {
+	req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
+	req.Body = &unreadableBody{t: t}
+	req.ContentLength = maxBodyRewriteSize + 1
+
+	if applyBodySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)}) {
+		t.Fatal("reported a substitution on a body it should not have touched")
+	}
+}
+
 // Dies mid-upload, the way a client that goes away does: some bytes, then an error.
 type halfBody struct {
 	data  []byte

From 13739026fedfcb04e846a058904803340cb981b5 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 19:39:29 +0530
Subject: [PATCH 07/25] test(agent-vault): pin the prefix boundary at depth

`/repos` not covering `/repositories` was pinned; `/repos/octo` not covering
`/repos/octopus` was not. It was the one case the TypeScript matcher's test had
that this file did not, and that matcher is being removed as a copy of a rule
that only runs here.
---
 packages/agentvault/policy_test.go | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
index b1f51be4..2bc7725c 100644
--- a/packages/agentvault/policy_test.go
+++ b/packages/agentvault/policy_test.go
@@ -124,6 +124,17 @@ func TestPathPolicy(t *testing.T) {
 		}
 	})
 
+	// The boundary holds at any depth, not only against the first segment.
+	t.Run("a deeper prefix still matches whole segments only", func(t *testing.T) {
+		deep := serviceWithPolicy(nil, []string{"/repos/octo"})
+		if err := checkServicePolicy(deep, requestTo(t, "GET", "/repos/octo/hello")); err != nil {
+			t.Fatalf("/repos/octo should cover /repos/octo/hello: %v", err)
+		}
+		if err := checkServicePolicy(deep, requestTo(t, "GET", "/repos/octopus")); err == nil {
+			t.Fatal("/repos/octo should not cover /repos/octopus")
+		}
+	})
+
 	t.Run("prefix / matches everything", func(t *testing.T) {
 		root := serviceWithPolicy(nil, []string{"/"})
 		if err := checkServicePolicy(root, requestTo(t, "GET", "/anything/at/all")); err != nil {

From 8a39d8bdfe922bd0bc85d8ee9b52fb71a9dce5ef Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 22:15:42 +0530
Subject: [PATCH 08/25] chore(agent-vault): cut the comments that narrate the
 code

CLAUDE.md asks for no comments by default, and one earns its place only by
explaining why: a non-obvious constraint, an ordering dependency, or logic that
looks wrong until you know the reason. This branch added 160 comment lines to
packages/agentvault, most of them retelling the line below.

Gone: the doc comment restating a sentinel's declaration, the four-line account of
where applySubstitutions was ported from, the seventeen-line essay above
pathAllowedAfterSubstitution, and the fixture preamble describing its own return
value. Kept, shorter: why ';' and '\' are traversal on Tomcat and IIS, why the
UTF-8 check beats refusing every high byte, why ContentLength is left disagreeing,
why the credential is injected last.

160 lines down to 92.
---
 packages/agentvault/policy.go                 | 61 +++++--------------
 packages/agentvault/policy_test.go            | 10 +--
 packages/agentvault/proxy.go                  | 23 +++----
 packages/agentvault/proxy_policy_test.go      | 28 +++------
 packages/agentvault/rewrite.go                | 56 +++++++----------
 .../rewrite_transformations_test.go           |  8 +--
 6 files changed, 58 insertions(+), 128 deletions(-)

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index b797b8b0..1d452b10 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -8,13 +8,9 @@ import (
 	"unicode/utf8"
 )
 
-// errPolicyBlocked is the sentinel for a service's own method and path rules, distinct from errHostBlocked,
-// which is the proxy-wide traffic policy. Both render as a 403 whose body is err.Error().
 var errPolicyBlocked = errors.New("blocked by service policy")
 
-// checkServicePolicy runs on the request exactly as it arrived, before anything rewrites it, and before the
-// plaintext refusal that nils a match: a restriction has to hold on http:// too, not only where a credential
-// would have been attached.
+// Runs before the plaintext refusal that nils a match, so a restriction holds on http:// too.
 func checkServicePolicy(svc *resolvedService, req *http.Request) error {
 	if !svc.allowsMethod(req.Method) {
 		return fmt.Errorf("service %q does not allow %s: %w", svc.name, req.Method, errPolicyBlocked)
@@ -28,8 +24,7 @@ func checkServicePolicy(svc *resolvedService, req *http.Request) error {
 	return nil
 }
 
-// A nil map is every method. The set is built upper-case, and the comparison folds the request's method the
-// same way, so a client sending "get" is judged on GET rather than silently blocked.
+// A nil map is every method. Folding both sides means a client sending "get" is judged on GET.
 func (s *resolvedService) allowsMethod(method string) bool {
 	if s.allowedMethods == nil {
 		return true
@@ -37,8 +32,7 @@ func (s *resolvedService) allowsMethod(method string) bool {
 	return s.allowedMethods[strings.ToUpper(method)]
 }
 
-// EscapedPath is byte-for-byte what Request.write puts on the wire (RequestURI() returns it, and forward only
-// rewrites Scheme, Host and RequestURI), so this judges exactly what the upstream will receive.
+// EscapedPath is what goes on the wire, so this judges what the upstream will receive.
 func requestPath(req *http.Request) string {
 	path := req.URL.EscapedPath()
 	if path == "" {
@@ -58,9 +52,8 @@ func truncatePath(path string) string {
 	return path
 }
 
-// pathAllowed never decodes. Anything whose meaning would depend on how the upstream normalises it is refused
-// outright, so the prefix comparison below is a plain byte comparison and the filter can only ever allow a
-// path every reader agrees on. Prefixes carry none of these characters by grammar.
+// Never decodes: anything whose meaning depends on the upstream's normalisation is refused outright, so the
+// comparison below is a plain byte comparison.
 func pathAllowed(escaped string, prefixes []string) bool {
 	if isAmbiguousPath(escaped) {
 		return false
@@ -68,23 +61,10 @@ func pathAllowed(escaped string, prefixes []string) bool {
 	return matchesPrefix(escaped, prefixes)
 }
 
-// pathAllowedAfterSubstitution judges a path the proxy itself part-wrote, so it cannot use the rule above.
-// applySubstitutions percent-escapes the value precisely so a secret containing '/' cannot add a segment,
-// and that escape is the '%2F' isAmbiguousPath refuses: judged by pathAllowed, a GitLab project addressed
-// as `group%2Fproject` would 403 against a prefix that plainly covers it.
-//
-// The prefix comparison is unchanged and still byte-exact, which is what keeps the substituted span after
-// the prefix: a placeholder sitting inside the prefix region rewrites those bytes and fails the comparison.
-// That leaves traversal as the only way out of an allowed prefix, so traversal is what is still refused,
-// judged on the decoded path because an upstream that decodes '%2F' before routing is exactly the reader
-// `..%2F..%2Fadmin` is written for.
-//
-// Traversal is not only a bare `..` segment. ';' and '\' are refused here for the same reason
-// isAmbiguousPath refuses them: Tomcat and Jetty strip `;params` per segment and IIS reads '\' as a
-// separator, so `..;x` and `..\admin` both walk up on some upstream while reading as an ordinary segment
-// to a splitter. The escape-shape checks isAmbiguousPath also runs are deliberately not repeated, since
-// the substituted span is percent-escaped by applySubstitutions and it is the decoded meaning that matters
-// here; running them on the decoded form would refuse a secret merely containing a '%'.
+// The path here is part-written by us: applySubstitutions escapes the value so it cannot add a segment, and
+// pathAllowed would refuse that very '%2F'. Only traversal can leave an allowed prefix, so only traversal is
+// refused, judged on the decoded path because that is what an upstream decoding '%2F' will route on. ';' and
+// '\' count as traversal here for the reason isAmbiguousPath gives.
 func pathAllowedAfterSubstitution(escaped, decoded string, prefixes []string) bool {
 	if strings.ContainsAny(decoded, ";\\") {
 		return false
@@ -114,9 +94,8 @@ func matchesPrefix(escaped string, prefixes []string) bool {
 }
 
 func isAmbiguousPath(escaped string) bool {
-	// ';' because Tomcat, Jetty and Spring strip ;params per segment before normalising, so /repos/..;/admin
-	// resolves to /admin upstream while reading as an ordinary segment here. '\' because Go treats it as a
-	// path byte and IIS and .NET read it as a separator.
+	// Tomcat and Spring strip ;params before normalising, so /repos/..;/admin resolves to /admin upstream
+	// while reading as an ordinary segment here. IIS reads '\' as a separator.
 	if strings.ContainsAny(escaped, ";\\") {
 		return true
 	}
@@ -128,20 +107,13 @@ func isAmbiguousPath(escaped string) bool {
 			return true
 		}
 	}
-	// An empty segment: /a//b normalises differently per server. A leading // is covered too; it could only
-	// ever fail the prefix comparison anyway, but judging it here keeps the rule one sentence.
+	// /a//b normalises differently per server.
 	return strings.Contains(escaped, "//")
 }
 
-// Judges the percent-escapes in a path. An escape is unsafe when it decodes to a separator, a dot, a
-// control byte, or to a byte sequence that is not valid UTF-8.
-//
-// The UTF-8 check is what lets a real non-ASCII path through while still refusing the attack it protects
-// against. `%c3%a9` is a correctly encoded 'é' and decodes to one rune; `%c0%ae` is an overlong encoding
-// of '.', which Go decodes to RuneError and some servers read as a dot. Rejecting every byte >= 0x80
-// would catch the second but also break every API that carries a filename or a user string in its path.
-// The rest (%2e, %2f, %5c, the double-encoded %252e, and the null-truncation ..%00) fall out of the
-// decoded-byte switch. %20 still works.
+// The UTF-8 check is what lets a real non-ASCII path through while still refusing the attack: `%c0%ae` is
+// an overlong '.', which some servers read as a dot, while `%c3%a9` is a legitimate 'é'. Refusing every
+// byte >= 0x80 would catch the first and break every API carrying a filename in its path.
 func hasUnsafeEscape(escaped string) bool {
 	decoded := make([]byte, 0, len(escaped))
 	sawEscape := false
@@ -173,8 +145,7 @@ func hasUnsafeEscape(escaped string) bool {
 		i += 2
 	}
 
-	// Only escaped input can carry an overlong or truncated sequence; an unescaped path is whatever the
-	// client put on the wire and is compared byte for byte anyway.
+	// Only escaped input can carry an overlong sequence.
 	return sawEscape && !utf8.Valid(decoded)
 }
 
diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
index 2bc7725c..c24db02c 100644
--- a/packages/agentvault/policy_test.go
+++ b/packages/agentvault/policy_test.go
@@ -161,8 +161,7 @@ func TestPathPolicy(t *testing.T) {
 
 func TestControlByteEscapesAreRefused(t *testing.T) {
 	svc := serviceWithPolicy(nil, []string{"/repos"})
-	// ..%00 is the null-truncation traversal: an upstream that decodes then truncates at NUL, or strips
-	// control bytes before normalising, reads this as /repos/.. and resolves outside the prefix.
+	// An upstream that truncates at NUL reads this as /repos/.. and resolves outside the prefix.
 	for _, path := range []string{"/repos/..%00/admin", "/repos/%00../admin", "/repos/x%09y", "/repos/x%7f"} {
 		t.Run(path, func(t *testing.T) {
 			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); !errors.Is(err, errPolicyBlocked) {
@@ -200,7 +199,6 @@ func TestWireMappingFailsClosed(t *testing.T) {
 func TestNonAsciiPathsAreJudgedByUtf8Validity(t *testing.T) {
 	svc := serviceWithPolicy(nil, []string{"/repos"})
 
-	// Correctly encoded UTF-8 is an ordinary path: é, 日本語, and an emoji all reach the upstream.
 	allowed := []string{
 		"/repos/owner/repo/contents/caf%C3%A9.md",
 		"/repos/%E6%97%A5%E6%9C%AC%E8%AA%9E",
@@ -215,8 +213,7 @@ func TestNonAsciiPathsAreJudgedByUtf8Validity(t *testing.T) {
 		})
 	}
 
-	// Overlong and malformed sequences stay blocked: %c0%ae is an overlong '.', which some servers
-	// normalise as a traversal segment.
+	// %c0%ae is an overlong '.', which some servers normalise as a traversal segment.
 	blocked := []string{
 		"/repos/%c0%ae%c0%ae/admin",
 		"/repos/%c0%af",
@@ -234,8 +231,7 @@ func TestNonAsciiPathsAreJudgedByUtf8Validity(t *testing.T) {
 }
 
 func TestAnExplicitRootPrefixMatchesEverythingAnUnrestrictedServiceWould(t *testing.T) {
-	// Setting "/" should mean the same as setting no prefix at all, which was not true while every
-	// high byte was refused outright.
+	// Setting "/" must mean the same as setting no prefix at all.
 	root := serviceWithPolicy(nil, []string{"/"})
 	open := serviceWithPolicy(nil, nil)
 
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index cd27dcdc..7f9f299a 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -376,8 +376,6 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 		decision, status, body = decisionError, http.StatusBadGateway, "failed to resolve the session"
 	case err != nil:
 		decision, status, body = decisionError, http.StatusBadGateway, "failed to reach the upstream"
-	// brokered means something was attached or rewritten, not merely that a service matched. A pass-through
-	// service carrying custom headers or substitutions counts.
 	case outcome.brokered:
 		decision, status = decisionBrokered, resp.StatusCode
 	default:
@@ -401,8 +399,8 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 	if matched != nil {
 		event = event.Str("service", matched.name).Str("accessBundle", matched.accessBundleName)
 	}
-	// Names the surfaces a placeholder was actually swapped in. Without it a substitution that matched
-	// nothing is indistinguishable from one that did: the logged path is the agent's either way.
+	// The logged path is always the agent's, so without this a substitution that matched nothing reads
+	// exactly like one that fired.
 	if len(outcome.substituted) > 0 {
 		event = event.Strs("substituted", outcome.substituted)
 	}
@@ -447,8 +445,7 @@ func (ps *proxyServer) blocksOffBundle(matched *resolvedService, hostname, port
 	return matched == nil && ps.currentConfig().TrafficPolicy == TrafficPolicyBundleHosts && !ps.isAllowedHost(hostname, port)
 }
 
-// What forward did to the request, for the log line. `brokered` is wider than "a credential went out": a
-// pass-through service carrying custom headers or substitutions is still brokering something.
+// `brokered` is wider than "a credential went out": custom headers or substitutions count too.
 type forwardOutcome struct {
 	brokered    bool
 	substituted []string
@@ -468,8 +465,7 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 		return nil, nil, outcome, fmt.Errorf("no service covers host %q: %w", hostname, errHostBlocked)
 	}
 
-	// Judged on the request as it arrived, and above the plaintext refusal below, so a method or path rule
-	// holds on http:// too rather than only where a credential would have been attached.
+	// Above the plaintext refusal below, so a rule holds on http:// too.
 	if matched != nil {
 		if err := checkServicePolicy(matched, req); err != nil {
 			return nil, matched, outcome, err
@@ -495,11 +491,8 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 				Msg("agent-vault: refusing to attach a credential over plaintext http")
 			matched = nil
 		} else {
-			// Substitutions run first so an injected real value can never itself be rewritten. The credential
-			// goes last and wins: a custom header naming the credential's own header would otherwise replace
-			// the real token with the custom value, and the agent would get a 401 nothing in the service
-			// explains. The backend refuses that pairing on write, so this is the floor under it rather than
-			// the only guard.
+			// Substitutions first, so an injected real value can never itself be rewritten. The credential last,
+			// so a custom header naming the credential's own header loses rather than replacing the token.
 			outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
 			outcome.brokered = injectCustomHeaders(req, matched.customHeaders)
 			if injectCredential(req, &matched.credential) {
@@ -509,12 +502,10 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 				outcome.brokered = true
 			}
 
-			// A path-surface substitution rewrites the path after the check above, so a restricted service
-			// re-checks what actually goes on the wire.
+			// The substitution above rewrote the path, so re-check what actually goes on the wire.
 			if len(matched.allowedPathPrefixes) > 0 && containsSurface(outcome.substituted, surfacePath) {
 				if !pathAllowedAfterSubstitution(requestPath(req), req.URL.Path, matched.allowedPathPrefixes) {
 					// The path now carries the real credential, so it must not reach the body or the log.
-					// Every other refusal in this file is fixed text for the same reason.
 					return nil, matched, outcome, fmt.Errorf(
 						"service %q does not allow the path this request substitutes to: %w",
 						matched.name, errPolicyBlocked,
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index a02d9bd9..1e75ccef 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -17,7 +17,6 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
-// What the upstream actually received, so the assertions are about the wire rather than our own structs.
 type echoed struct {
 	Method  string              `json:"method"`
 	Path    string              `json:"path"`
@@ -32,11 +31,9 @@ func (r fixedResolver) resolve(string) (*resolveResult, error) {
 	return &resolveResult{SessionID: "s1", Services: r.services}, nil
 }
 
-// Stands up the whole path an agent's request takes: CONNECT to the proxy, TLS terminated by the proxy's
-// own CA, policy and injection applied, then forwarded over TLS to a real upstream that echoes what it got.
-// The upstream is addressed as 127.0.0.1, which is what httptest's certificate carries and what mintLeaf
-// puts in an IP SAN, so both TLS legs verify and the CONNECT target the proxy dials is the upstream itself.
-// Returns the client and the service's host, ready to build a URL from.
+// The whole path an agent's request takes: CONNECT, TLS terminated by the proxy's own CA, policy and
+// injection applied, then forwarded to a real upstream that echoes what it got. The upstream is addressed as
+// 127.0.0.1 so both TLS legs verify against httptest's certificate and mintLeaf's IP SAN.
 func newPolicyFixture(t *testing.T, build func(host string) *resolvedService) (*http.Client, string) {
 	t.Helper()
 
@@ -68,7 +65,6 @@ func newPolicyFixture(t *testing.T, build func(host string) *resolvedService) (*
 	}
 
 	transport := newUpstreamTransport()
-	// The proxy has to trust the httptest upstream's self-signed certificate to reach it.
 	transport.TLSClientConfig = &tls.Config{RootCAs: upstreamPool}
 
 	ps := &proxyServer{transport: transport, ca: newCaManager(key, cert)}
@@ -244,8 +240,7 @@ func TestSubstitutionsReachTheUpstream(t *testing.T) {
 }
 
 func TestABlockedSubstitutedPathNeverEchoesTheSecret(t *testing.T) {
-	// The re-check happens after the real value is in the path, so the refusal must not quote the path:
-	// the 403 body goes back to the agent and the same text goes to the proxy log.
+	// The 403 body goes back to the agent and the same text goes to the proxy log.
 	secret := "../s3cr3tadmin"
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
@@ -263,8 +258,7 @@ func TestABlockedSubstitutedPathNeverEchoesTheSecret(t *testing.T) {
 }
 
 func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
-	// The path is authorised before substitution, so a value containing a traversal must not smuggle the
-	// request out of its prefix afterwards.
+	// The path is authorised before substitution, so a traversal must not smuggle the request out after it.
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
 			subOn("__PAT__", "../admin", surfacePath),
@@ -277,8 +271,7 @@ func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
 	}
 }
 
-// `..` alone is not the only way out of a prefix: some upstreams read ';' and '\\' as separators, so a
-// value glued onto an agent-supplied `..` walks up there while reading as an ordinary segment to a splitter.
+// Some upstreams read ';' and '\\' as separators, so a value glued onto an agent-supplied `..` walks up.
 func TestASubstitutedValueCannotWalkOutOfItsPrefix(t *testing.T) {
 	for _, secret := range []string{`\admin`, `;x`, `/admin`} {
 		t.Run(secret, func(t *testing.T) {
@@ -297,8 +290,7 @@ func TestASubstitutedValueCannotWalkOutOfItsPrefix(t *testing.T) {
 }
 
 func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
-	// applySubstitutions escapes the value so it cannot add a segment, and that escape must not then read
-	// as the ambiguity it was written to prevent: a GitLab project is addressed as `group%2Fproject`.
+	// The escape that stops a value adding a segment must not then read as the ambiguity it prevents.
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/api/v4/projects"}, nil, []substitution{
 			subOn("__PROJ__", "mygroup/myproject", surfacePath),
@@ -314,8 +306,7 @@ func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
 	}
 }
 
-// The backend refuses this pairing on write, so reaching it means a service saved before that check or a
-// write that raced it. Either way the real token has to survive.
+// The backend refuses this pairing on write, so reaching it means a stale service. The token has to survive.
 func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
 	cases := []struct {
 		name          string
@@ -372,8 +363,7 @@ func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
 }
 
 func TestTheLogSaysWhichSurfacesWereSubstituted(t *testing.T) {
-	// The logged path is always the agent's own, placeholder and all, so without this field a
-	// substitution that matched nothing reads exactly like one that fired.
+	// The logged path is always the agent's own, so without this field a miss reads like a hit.
 	type line struct {
 		Path        string   `json:"path"`
 		Decision    string   `json:"decision"`
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 249bcb34..be392891 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -48,9 +48,8 @@ func injectCredential(req *http.Request, cred *credential) bool {
 	}
 }
 
-// injectCustomHeaders writes the service's custom headers, before the credential, so one whose name collides with
-// the credential's loses to it rather than replacing the real token. A pass-through service injects no
-// credential at all, which is why setting Authorization as a custom header on one still works.
+// Written before the credential, so one colliding with the credential's header loses to it. Pass-through
+// injects nothing, which is why Authorization as a custom header on one still works.
 func injectCustomHeaders(req *http.Request, customHeaders []customHeader) bool {
 	for _, header := range customHeaders {
 		value := string(header.value)
@@ -62,10 +61,8 @@ func injectCustomHeaders(req *http.Request, customHeaders []customHeader) bool {
 	return len(customHeaders) > 0
 }
 
-// applySubstitutions swaps each placeholder for its real value across the surfaces the service names.
-// Ported from the agent proxy (packages/agentproxy/rewrite.go), with one deliberate change: a body it
-// cannot rewrite is logged rather than skipped in silence, because the request then goes upstream with the
-// placeholder still in it and the agent only ever sees a third-party 401.
+// A body it cannot rewrite is logged rather than skipped in silence: the placeholder goes upstream and the
+// agent would otherwise see only a third-party 401.
 func applySubstitutions(req *http.Request, serviceName string, subs []substitution) []string {
 	changed := map[string]bool{}
 	for _, sub := range subs {
@@ -74,16 +71,13 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 		}
 		real := string(sub.value)
 
-		// Swapped in the escaped path and written back escaped, so every other segment keeps the byte form
-		// the agent sent. Rewriting the decoded Path and clearing RawPath (which is what Agent Proxy does)
-		// makes Go re-derive the wire path, and its encoder does not re-escape '/' or '+': a GitLab project
-		// addressed as `group%2Fproject` would arrive as two segments, pointing at a different resource.
+		// Swapped in the escaped path so every other segment keeps the byte form the agent sent. Rewriting the
+		// decoded Path makes Go re-derive the wire path without re-escaping '/', and `group%2Fproject` would
+		// arrive as two segments pointing at a different resource.
 		if sub.surfaces[surfacePath] {
 			escaped := req.URL.EscapedPath()
-			// EscapedPath re-encodes the whole path whenever what the agent sent is not already valid
-			// encoding, so a `{{TOKEN}}` on the wire reads here as `%7B%7BTOKEN%7D%7D`. Looking only for the
-			// form the author typed would leave those placeholders on the wire, and the agent would see
-			// nothing but a third-party 404.
+			// EscapedPath re-encodes the whole path when what the agent sent is not already valid encoding, so
+			// `{{TOKEN}}` reads here as `%7B%7BTOKEN%7D%7D` and matching only the typed form would miss it.
 			needle := sub.placeholder
 			if !strings.Contains(escaped, needle) {
 				needle = escapedPathForm(sub.placeholder)
@@ -92,8 +86,7 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 				// The replacement is escaped too, or a secret containing '/' or '?' would itself reshape the URL.
 				if v, ok := replaceWithinLimit(escaped, needle, url.PathEscape(real), maxBodyRewriteSize); ok {
 					if decoded, err := url.PathUnescape(v); err == nil {
-						// Both halves: Path is what the policy re-check and any later reader see, RawPath is
-						// what goes on the wire. Go uses RawPath only when it agrees with Path.
+						// Go uses RawPath only when it agrees with Path, so both are written.
 						req.URL.Path = decoded
 						req.URL.RawPath = v
 						changed[surfacePath] = true
@@ -141,9 +134,8 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 	return surfaces
 }
 
-// escapedPathForm is the placeholder as EscapedPath would render it: the same encoder, reached the same way.
-// The leading '/' keeps url.URL's `Path == "*"` special case out of it, and the path encoder leaves a slash
-// alone, so trimming it back off is exact.
+// The placeholder as EscapedPath would render it. The leading '/' keeps url.URL's `Path == "*"` case out of
+// it, and the encoder leaves a slash alone, so trimming it back off is exact.
 func escapedPathForm(placeholder string) string {
 	return strings.TrimPrefix((&url.URL{Path: "/" + placeholder}).EscapedPath(), "/")
 }
@@ -157,8 +149,7 @@ func bodySubstitutions(subs []substitution) bool {
 	return false
 }
 
-// The body is only ever read when some substitution names it, so a service without one keeps streaming
-// exactly as before.
+// Only read when a substitution names the body, so every other service keeps streaming.
 func applyBodySubstitutions(req *http.Request, serviceName string, subs []substitution) bool {
 	if req.Body == http.NoBody || req.ContentLength == 0 {
 		return false
@@ -170,9 +161,8 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 			Msg("agent-vault: body substitution skipped on an encoded body; the placeholder is going upstream unchanged")
 		return false
 	}
-	// Judged before reading, so a body that already says it is too big costs no memory at all. The check
-	// below still has to stand on its own: a chunked request declares -1, and a declared length is the
-	// client's claim rather than a fact.
+	// Judged before reading, so an oversize body costs no memory. The check below still has to stand on its
+	// own: a chunked request declares -1, and a declared length is a claim rather than a fact.
 	if req.ContentLength > maxBodyRewriteSize {
 		log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
 			Int64("declaredBytes", req.ContentLength).
@@ -182,11 +172,9 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 
 	body, err := io.ReadAll(io.LimitReader(req.Body, maxBodyRewriteSize+1))
 	if err != nil {
-		// Deliberately not correcting ContentLength to what was actually read. The declared length and the
-		// bytes now disagree, so http.Transport refuses the request and nothing reaches the upstream. Making
-		// them agree would hand the upstream a well-formed shorter request it cannot tell from a complete
-		// one, turning a broken upload into a partial write nobody can take back. A 502 the agent may not
-		// even be alive to see is the better end of that trade.
+		// ContentLength is deliberately left disagreeing with the bytes, so http.Transport refuses the request.
+		// Correcting it would hand the upstream a well-formed shorter request it cannot tell from a complete
+		// one, turning a broken upload into a partial write nobody can take back.
 		_ = req.Body.Close()
 		req.Body = io.NopCloser(bytes.NewReader(body))
 		log.Warn().Err(err).Str("service", serviceName).Int("bytesRead", len(body)).
@@ -233,9 +221,8 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 	return replaced
 }
 
-// replaceWithinLimit substitutes every occurrence of old in s, but only when the expanded result stays
-// within limit bytes; otherwise it returns the input unchanged. This stops a short placeholder mapped to a
-// long secret from ballooning proxy memory, since ReplaceAll allocates by the expansion ratio.
+// Returns the input unchanged when the expansion would exceed limit, so a short placeholder mapped to a long
+// secret cannot balloon proxy memory.
 func replaceWithinLimit(s, old, replacement string, limit int) (string, bool) {
 	count := strings.Count(s, old)
 	if count == 0 {
@@ -251,8 +238,7 @@ func replaceWithinLimit(s, old, replacement string, limit int) (string, bool) {
 // Callers strip before injecting a credential: a Connection list naming the credential's header would
 // otherwise delete it, which is the same trap Go documents on httputil.ReverseProxy.Director.
 func stripHopByHopHeaders(header http.Header) {
-	// Connection names the headers meant for this hop alone, so read it before deleting it. Deleting it
-	// first would forward the marked header with nothing left to say it was hop-by-hop.
+	// Read Connection before deleting it, or the headers it names go on with nothing marking them.
 	for _, values := range header.Values("Connection") {
 		for _, name := range strings.Split(values, ",") {
 			if name = strings.TrimSpace(name); name != "" {
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 797569fc..4785ca50 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -65,8 +65,7 @@ func (b *unreadableBody) Read([]byte) (int, error) {
 
 func (b *unreadableBody) Close() error { return nil }
 
-// Reading first and measuring afterwards means every oversize request costs the cap in memory before it is
-// refused, which with the connection limit is several gigabytes an agent can make the proxy hold.
+// Measuring after reading would cost the cap in memory per request before refusing it.
 func TestAnOversizedDeclaredBodyIsNeverRead(t *testing.T) {
 	req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
 	req.Body = &unreadableBody{t: t}
@@ -95,8 +94,7 @@ func (b *halfBody) Read(p []byte) (int, error) {
 
 func (b *halfBody) Close() error { return nil }
 
-// Correcting the length here would hand the upstream a well-formed shorter request it cannot tell from a
-// complete one. Leaving the two disagreeing is what makes http.Transport refuse to send anything.
+// Correcting the length would hand the upstream a shorter request it cannot tell from a complete one.
 func TestABrokenUploadIsNotForwardedTruncated(t *testing.T) {
 	full := strings.Repeat("A", 500) + "__PAT__" + strings.Repeat("B", 500)
 	req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
@@ -185,7 +183,6 @@ func TestApplySubstitutions(t *testing.T) {
 		}
 	})
 
-	// The placeholder goes upstream unchanged here, which is why the proxy logs it rather than staying quiet.
 	t.Run("an encoded body is forwarded untouched", func(t *testing.T) {
 		body := `{"token":"__TOKEN__"}`
 		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
@@ -241,7 +238,6 @@ func TestApplySubstitutions(t *testing.T) {
 
 func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
 	// GitLab addresses a project as group%2Fproject: one name containing a slash, not two segments.
-	// Re-deriving the wire path from the decoded Path would turn it into two and address a different repo.
 	for _, tc := range []struct{ name, target, wantURI string }{
 		{
 			"an encoded slash survives",

From a8ecf561f5840a3789203430e65267f6ecfb6580 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Tue, 15 Sep 2026 22:34:35 +0530
Subject: [PATCH 09/25] chore(agent-vault): delete the comments that say what
 the code says

CLAUDE.md asks for no comments by default, and one earns its place only by
explaining why: a non-obvious constraint, a workaround, an ordering dependency, or
logic that looks wrong until you know the reason. This branch had 379 comment lines
across 4,600 added ones, and most of them failed that test.

Gone: every "null means unrestricted" beside a `| null` type, the notes telling a
test what its own name already says, the accounts of which query runs where, the
fixture preamble describing its own return value, and the paragraphs explaining an
optimisation nobody would undo by accident.

Kept, and shortened: why ';' and '\' count as traversal on Tomcat and IIS, why the
UTF-8 check beats refusing every high byte, why ContentLength is left disagreeing
with the body, why the credential is injected last, why removal is Base UI's and
adding is ours, why the shadow check reads both halves on the primary, and why the
rows are read before the delete that cascades them away.

379 down to 180.
---
 packages/agentvault/policy.go                       | 4 ----
 packages/agentvault/policy_test.go                  | 7 -------
 packages/agentvault/proxy.go                        | 3 ---
 packages/agentvault/proxy_policy_test.go            | 9 ---------
 packages/agentvault/resolve.go                      | 2 --
 packages/agentvault/rewrite.go                      | 4 ----
 packages/agentvault/rewrite_transformations_test.go | 8 --------
 7 files changed, 37 deletions(-)

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index 1d452b10..eade5b77 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -24,7 +24,6 @@ func checkServicePolicy(svc *resolvedService, req *http.Request) error {
 	return nil
 }
 
-// A nil map is every method. Folding both sides means a client sending "get" is judged on GET.
 func (s *resolvedService) allowsMethod(method string) bool {
 	if s.allowedMethods == nil {
 		return true
@@ -32,7 +31,6 @@ func (s *resolvedService) allowsMethod(method string) bool {
 	return s.allowedMethods[strings.ToUpper(method)]
 }
 
-// EscapedPath is what goes on the wire, so this judges what the upstream will receive.
 func requestPath(req *http.Request) string {
 	path := req.URL.EscapedPath()
 	if path == "" {
@@ -85,7 +83,6 @@ func matchesPrefix(escaped string, prefixes []string) bool {
 		if !strings.HasPrefix(escaped, prefix) {
 			continue
 		}
-		// Whole segments only, so /repos does not cover /repositories.
 		if rest := escaped[len(prefix):]; rest == "" || rest[0] == '/' {
 			return true
 		}
@@ -124,7 +121,6 @@ func hasUnsafeEscape(escaped string) bool {
 			continue
 		}
 		if i+2 >= len(escaped) {
-			// A truncated escape is not something we can judge either.
 			return true
 		}
 		hi, hiOk := unhex(escaped[i+1])
diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
index c24db02c..2329923b 100644
--- a/packages/agentvault/policy_test.go
+++ b/packages/agentvault/policy_test.go
@@ -43,7 +43,6 @@ func TestMethodPolicy(t *testing.T) {
 		if !errors.Is(err, errPolicyBlocked) {
 			t.Fatalf("POST should be blocked, got %v", err)
 		}
-		// The body is err.Error(), so the service and the method both have to be in it.
 		if !strings.Contains(err.Error(), `service "github" does not allow POST`) {
 			t.Fatalf("unhelpful message: %q", err.Error())
 		}
@@ -71,7 +70,6 @@ func TestPathPolicy(t *testing.T) {
 		})
 	}
 
-	// Each of these reads as inside /repos to a naive prefix check but resolves elsewhere on some upstream.
 	blocked := []string{
 		"/repositories",
 		"/repo",
@@ -92,7 +90,6 @@ func TestPathPolicy(t *testing.T) {
 	for _, path := range blocked {
 		t.Run("blocks "+path, func(t *testing.T) {
 			req := requestTo(t, "GET", "/placeholder")
-			// Set the target verbatim so Go's URL parsing cannot normalise the case away before we see it.
 			req.URL.Path = ""
 			req.URL.RawPath = ""
 			req.URL.Opaque = ""
@@ -124,7 +121,6 @@ func TestPathPolicy(t *testing.T) {
 		}
 	})
 
-	// The boundary holds at any depth, not only against the first segment.
 	t.Run("a deeper prefix still matches whole segments only", func(t *testing.T) {
 		deep := serviceWithPolicy(nil, []string{"/repos/octo"})
 		if err := checkServicePolicy(deep, requestTo(t, "GET", "/repos/octo/hello")); err != nil {
@@ -161,7 +157,6 @@ func TestPathPolicy(t *testing.T) {
 
 func TestControlByteEscapesAreRefused(t *testing.T) {
 	svc := serviceWithPolicy(nil, []string{"/repos"})
-	// An upstream that truncates at NUL reads this as /repos/.. and resolves outside the prefix.
 	for _, path := range []string{"/repos/..%00/admin", "/repos/%00../admin", "/repos/x%09y", "/repos/x%7f"} {
 		t.Run(path, func(t *testing.T) {
 			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); !errors.Is(err, errPolicyBlocked) {
@@ -184,7 +179,6 @@ func TestWireMappingFailsClosed(t *testing.T) {
 			t.Fatalf("an empty method list must restrict, got %v", methods)
 		}
 
-		// A list whose entries are all blank must not collapse to "unrestricted".
 		prefixes := toPathPrefixes([]string{"  "})
 		if len(prefixes) == 0 {
 			t.Fatal("an empty path prefix list must restrict, not fall through to unrestricted")
@@ -231,7 +225,6 @@ func TestNonAsciiPathsAreJudgedByUtf8Validity(t *testing.T) {
 }
 
 func TestAnExplicitRootPrefixMatchesEverythingAnUnrestrictedServiceWould(t *testing.T) {
-	// Setting "/" must mean the same as setting no prefix at all.
 	root := serviceWithPolicy(nil, []string{"/"})
 	open := serviceWithPolicy(nil, nil)
 
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 7f9f299a..60672c41 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -445,7 +445,6 @@ func (ps *proxyServer) blocksOffBundle(matched *resolvedService, hostname, port
 	return matched == nil && ps.currentConfig().TrafficPolicy == TrafficPolicyBundleHosts && !ps.isAllowedHost(hostname, port)
 }
 
-// `brokered` is wider than "a credential went out": custom headers or substitutions count too.
 type forwardOutcome struct {
 	brokered    bool
 	substituted []string
@@ -465,7 +464,6 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 		return nil, nil, outcome, fmt.Errorf("no service covers host %q: %w", hostname, errHostBlocked)
 	}
 
-	// Above the plaintext refusal below, so a rule holds on http:// too.
 	if matched != nil {
 		if err := checkServicePolicy(matched, req); err != nil {
 			return nil, matched, outcome, err
@@ -502,7 +500,6 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 				outcome.brokered = true
 			}
 
-			// The substitution above rewrote the path, so re-check what actually goes on the wire.
 			if len(matched.allowedPathPrefixes) > 0 && containsSurface(outcome.substituted, surfacePath) {
 				if !pathAllowedAfterSubstitution(requestPath(req), req.URL.Path, matched.allowedPathPrefixes) {
 					// The path now carries the real credential, so it must not reach the body or the log.
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index 1e75ccef..ff0c1646 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -233,14 +233,12 @@ func TestSubstitutionsReachTheUpstream(t *testing.T) {
 	if got.Body != `{"token":"real-token"}` {
 		t.Fatalf("body = %q", got.Body)
 	}
-	// The placeholder must be gone from every surface, not merely replaced in the ones we checked.
 	if strings.Contains(string(payload), "__PAT__") {
 		t.Fatalf("a placeholder survived to the upstream: %s", payload)
 	}
 }
 
 func TestABlockedSubstitutedPathNeverEchoesTheSecret(t *testing.T) {
-	// The 403 body goes back to the agent and the same text goes to the proxy log.
 	secret := "../s3cr3tadmin"
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
@@ -258,7 +256,6 @@ func TestABlockedSubstitutedPathNeverEchoesTheSecret(t *testing.T) {
 }
 
 func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
-	// The path is authorised before substitution, so a traversal must not smuggle the request out after it.
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/repos"}, nil, []substitution{
 			subOn("__PAT__", "../admin", surfacePath),
@@ -271,7 +268,6 @@ func TestAPathSubstitutionIsRecheckedAgainstThePolicy(t *testing.T) {
 	}
 }
 
-// Some upstreams read ';' and '\\' as separators, so a value glued onto an agent-supplied `..` walks up.
 func TestASubstitutedValueCannotWalkOutOfItsPrefix(t *testing.T) {
 	for _, secret := range []string{`\admin`, `;x`, `/admin`} {
 		t.Run(secret, func(t *testing.T) {
@@ -290,7 +286,6 @@ func TestASubstitutedValueCannotWalkOutOfItsPrefix(t *testing.T) {
 }
 
 func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
-	// The escape that stops a value adding a segment must not then read as the ambiguity it prevents.
 	client, host := newPolicyFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, []string{"/api/v4/projects"}, nil, []substitution{
 			subOn("__PROJ__", "mygroup/myproject", surfacePath),
@@ -306,7 +301,6 @@ func TestAPathSubstitutionMayCarryASlashUnderAPrefix(t *testing.T) {
 	}
 }
 
-// The backend refuses this pairing on write, so reaching it means a stale service. The token has to survive.
 func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
 	cases := []struct {
 		name          string
@@ -330,7 +324,6 @@ func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
 			want:          "real-token",
 		},
 		{
-			// Pass-through injects no credential, so a custom Authorization header is the whole point.
 			name:          "pass-through leaves the custom header alone",
 			cred:          credential{kind: credentialPassthrough},
 			customHeaders: []customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("custom")}},
@@ -363,7 +356,6 @@ func TestACustomHeaderCannotReplaceTheCredential(t *testing.T) {
 }
 
 func TestTheLogSaysWhichSurfacesWereSubstituted(t *testing.T) {
-	// The logged path is always the agent's own, so without this field a miss reads like a hit.
 	type line struct {
 		Path        string   `json:"path"`
 		Decision    string   `json:"decision"`
@@ -405,7 +397,6 @@ func TestTheLogSaysWhichSurfacesWereSubstituted(t *testing.T) {
 		if len(got.Substituted) != 1 || got.Substituted[0] != surfacePath {
 			t.Fatalf("substituted = %v, want [path]", got.Substituted)
 		}
-		// The agent's own placeholder, never the value it was swapped for.
 		if got.Path != "/repos/__PAT__/x" {
 			t.Fatalf("path = %q", got.Path)
 		}
diff --git a/packages/agentvault/resolve.go b/packages/agentvault/resolve.go
index 20d12c8b..69260b72 100644
--- a/packages/agentvault/resolve.go
+++ b/packages/agentvault/resolve.go
@@ -89,8 +89,6 @@ func toCredential(wire api.AgentVaultCredential) credential {
 	}
 }
 
-// A nil slice stays a nil map, which allowsMethod reads as "every method". An empty list from the server
-// would be a restriction allowing nothing, so it is kept distinct rather than folded into nil.
 func toMethodSet(methods []string) map[string]bool {
 	if methods == nil {
 		return nil
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index be392891..4a3105d2 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -25,7 +25,6 @@ const (
 	maxBodyRewriteSize = 10 * 1024 * 1024
 )
 
-// injectCredential overwrites an existing header on the agent's request, silently and deliberately.
 func injectCredential(req *http.Request, cred *credential) bool {
 	switch cred.kind {
 	case credentialBearer:
@@ -83,7 +82,6 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 				needle = escapedPathForm(sub.placeholder)
 			}
 			if strings.Contains(escaped, needle) {
-				// The replacement is escaped too, or a secret containing '/' or '?' would itself reshape the URL.
 				if v, ok := replaceWithinLimit(escaped, needle, url.PathEscape(real), maxBodyRewriteSize); ok {
 					if decoded, err := url.PathUnescape(v); err == nil {
 						// Go uses RawPath only when it agrees with Path, so both are written.
@@ -149,7 +147,6 @@ func bodySubstitutions(subs []substitution) bool {
 	return false
 }
 
-// Only read when a substitution names the body, so every other service keeps streaming.
 func applyBodySubstitutions(req *http.Request, serviceName string, subs []substitution) bool {
 	if req.Body == http.NoBody || req.ContentLength == 0 {
 		return false
@@ -199,7 +196,6 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 		if count == 0 {
 			continue
 		}
-		// Forward unchanged when expanding the placeholder would push the body past the cap.
 		if len(rewritten)+count*(len(sub.value)-len(sub.placeholder)) > maxBodyRewriteSize {
 			log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
 				Msg("agent-vault: substituted body would exceed the limit; the placeholder is going upstream unchanged")
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 4785ca50..4cc9c8f4 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -43,7 +43,6 @@ func TestInjectCustomHeaders(t *testing.T) {
 		}
 	})
 
-	// Same trap injectCredential documents: a Connection list naming the header would delete it.
 	t.Run("a Connection header cannot delete an injected custom header", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
 		req.Header.Set("Connection", "X-Org-Id")
@@ -55,7 +54,6 @@ func TestInjectCustomHeaders(t *testing.T) {
 	})
 }
 
-// Fails the test if anything reads it.
 type unreadableBody struct{ t *testing.T }
 
 func (b *unreadableBody) Read([]byte) (int, error) {
@@ -65,7 +63,6 @@ func (b *unreadableBody) Read([]byte) (int, error) {
 
 func (b *unreadableBody) Close() error { return nil }
 
-// Measuring after reading would cost the cap in memory per request before refusing it.
 func TestAnOversizedDeclaredBodyIsNeverRead(t *testing.T) {
 	req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
 	req.Body = &unreadableBody{t: t}
@@ -76,7 +73,6 @@ func TestAnOversizedDeclaredBodyIsNeverRead(t *testing.T) {
 	}
 }
 
-// Dies mid-upload, the way a client that goes away does: some bytes, then an error.
 type halfBody struct {
 	data  []byte
 	n     int
@@ -94,7 +90,6 @@ func (b *halfBody) Read(p []byte) (int, error) {
 
 func (b *halfBody) Close() error { return nil }
 
-// Correcting the length would hand the upstream a shorter request it cannot tell from a complete one.
 func TestABrokenUploadIsNotForwardedTruncated(t *testing.T) {
 	full := strings.Repeat("A", 500) + "__PAT__" + strings.Repeat("B", 500)
 	req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
@@ -128,7 +123,6 @@ func TestApplySubstitutions(t *testing.T) {
 		}
 	})
 
-	// Go escapes '{' in a path, so EscapedPath carries the placeholder in a form the author never typed.
 	t.Run("path, placeholder Go re-encodes", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://gitlab.com/api/v4/projects/{{PROJECT}}/pipelines", nil)
 		surfaces := applySubstitutions(req, "gitlab", []substitution{subOn("{{PROJECT}}", "group/project", surfacePath)})
@@ -237,7 +231,6 @@ func TestApplySubstitutions(t *testing.T) {
 }
 
 func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
-	// GitLab addresses a project as group%2Fproject: one name containing a slash, not two segments.
 	for _, tc := range []struct{ name, target, wantURI string }{
 		{
 			"an encoded slash survives",
@@ -282,7 +275,6 @@ func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
 }
 
 func TestAQuerySubstitutionEscapesTheValue(t *testing.T) {
-	// secretValueSchema allows '+', '&' and spaces, and RawQuery goes on the wire verbatim.
 	for _, tc := range []struct{ name, secret, wantKey string }{
 		{"a base64 key with a plus", "aB+cD/eF==", "aB+cD/eF=="},
 		{"a value with a space", "has space", "has space"},

From a2d5e9ff23fac8074f7a7fbedace128ffbdb0e60 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Wed, 16 Sep 2026 00:37:54 +0530
Subject: [PATCH 10/25] fix(agent-vault): do not log a header value the
 substitution already filled

Header substitutions run before the body pass, so a request carrying
Content-Encoding: <placeholder> and a body reached this warning with the
real secret in it. The agent never held that secret and can read the
proxy's own log, so only the presence of an encoding is recorded now.
---
 packages/agentvault/rewrite.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 4a3105d2..68629416 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -151,10 +151,10 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 	if req.Body == http.NoBody || req.ContentLength == 0 {
 		return false
 	}
-	if enc := req.Header.Get("Content-Encoding"); enc != "" {
+	if req.Header.Get("Content-Encoding") != "" {
 		log.Warn().
 			Str("service", serviceName).
-			Str("contentEncoding", enc).
+			Bool("hasContentEncoding", true).
 			Msg("agent-vault: body substitution skipped on an encoded body; the placeholder is going upstream unchanged")
 		return false
 	}

From 9e57377388bfd0ecef2a0dafaa9ee8cc45deff80 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Thu, 17 Sep 2026 14:57:11 +0530
Subject: [PATCH 11/25] fix(agent-vault): let an all-slashes path prefix fail
 closed

The empty check ran before the trailing-slash trim, so "//" passed it and
then became "", which matches every path. A service restricted to /repos
allowed /admin instead, the opposite of what the comment above promises.
The server rejects "//" on write, so this was never live, but the proxy is
the enforcement point and revalidates nothing it is sent.
---
 packages/agentvault/policy_test.go | 17 ++++++++++-------
 packages/agentvault/resolve.go     |  7 ++++---
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
index 2329923b..277961a0 100644
--- a/packages/agentvault/policy_test.go
+++ b/packages/agentvault/policy_test.go
@@ -179,13 +179,16 @@ func TestWireMappingFailsClosed(t *testing.T) {
 			t.Fatalf("an empty method list must restrict, got %v", methods)
 		}
 
-		prefixes := toPathPrefixes([]string{"  "})
-		if len(prefixes) == 0 {
-			t.Fatal("an empty path prefix list must restrict, not fall through to unrestricted")
-		}
-		svc := &resolvedService{name: "s", allowedPathPrefixes: prefixes}
-		if err := checkServicePolicy(svc, requestTo(t, "GET", "/anything")); !errors.Is(err, errPolicyBlocked) {
-			t.Fatalf("expected a block, got %v", err)
+		// "//" trims to "" the same way "  " does, so both have to reach the fail-closed guard.
+		for _, empty := range []string{"  ", "//", "///"} {
+			prefixes := toPathPrefixes([]string{empty})
+			if len(prefixes) == 0 {
+				t.Fatalf("%q must restrict, not fall through to unrestricted", empty)
+			}
+			svc := &resolvedService{name: "s", allowedPathPrefixes: prefixes}
+			if err := checkServicePolicy(svc, requestTo(t, "GET", "/anything")); !errors.Is(err, errPolicyBlocked) {
+				t.Fatalf("%q: expected a block, got %v", empty, err)
+			}
 		}
 	})
 }
diff --git a/packages/agentvault/resolve.go b/packages/agentvault/resolve.go
index 69260b72..76add610 100644
--- a/packages/agentvault/resolve.go
+++ b/packages/agentvault/resolve.go
@@ -112,12 +112,13 @@ func toPathPrefixes(prefixes []string) []string {
 	out := make([]string, 0, len(prefixes))
 	for _, prefix := range prefixes {
 		prefix = strings.TrimSpace(prefix)
-		if prefix == "" {
-			continue
-		}
 		if prefix != "/" {
 			prefix = strings.TrimRight(prefix, "/")
 		}
+		// After the trim, so an all-slashes prefix cannot arrive here as "" and match every path.
+		if prefix == "" {
+			continue
+		}
 		out = append(out, prefix)
 	}
 	if len(out) == 0 {

From 3774b4481b8514da785100fbe29f84fa4f5fd225 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Thu, 17 Sep 2026 16:28:47 +0530
Subject: [PATCH 12/25] fix(agent-vault): swap the longest placeholder first

Substitutions ran in the order the server sent them, so a placeholder
starting with another one ate its prefix: with __TOKEN__ before
__TOKEN__V2, the header meant for the second secret went out carrying the
first with V2 glued on, and the second never sent. The same pair was
correct in the other order.

Sorted at resolve time rather than per request, since the slice is shared
by every request a session serves.
---
 packages/agentvault/resolve.go                |  7 ++++++
 .../rewrite_transformations_test.go           | 23 +++++++++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/packages/agentvault/resolve.go b/packages/agentvault/resolve.go
index 76add610..25e0d503 100644
--- a/packages/agentvault/resolve.go
+++ b/packages/agentvault/resolve.go
@@ -1,6 +1,7 @@
 package agentvault
 
 import (
+	"sort"
 	"strings"
 	"time"
 
@@ -151,5 +152,11 @@ func toSubstitutions(wire []api.AgentVaultSubstitution) []substitution {
 		}
 		subs = append(subs, substitution{placeholder: s.Placeholder, surfaces: surfaces, value: []byte(s.Value)})
 	}
+	// Longest first, so a placeholder that starts with another one is swapped before the shorter one can
+	// eat its prefix and leave the tail behind. Sorted here rather than per request: the slice is shared by
+	// every request the session serves.
+	sort.SliceStable(subs, func(i, j int) bool {
+		return len(subs[i].placeholder) > len(subs[j].placeholder)
+	})
 	return subs
 }
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 4cc9c8f4..091228d0 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -9,6 +9,8 @@ import (
 	"net/url"
 	"strings"
 	"testing"
+
+	"github.com/Infisical/infisical-merge/packages/api"
 )
 
 func subOn(placeholder, value string, surfaces ...string) substitution {
@@ -230,6 +232,27 @@ func TestApplySubstitutions(t *testing.T) {
 	})
 }
 
+// A placeholder that starts with another one is only swapped correctly when the longer runs first, and the
+// server is free to send them in either order.
+func TestAPlaceholderPrefixingAnotherStillSendsItsOwnSecret(t *testing.T) {
+	short := api.AgentVaultSubstitution{Placeholder: "__TOKEN__", Surfaces: []string{"header"}, Value: "SECRET_A"}
+	long := api.AgentVaultSubstitution{Placeholder: "__TOKEN__V2", Surfaces: []string{"header"}, Value: "SECRET_B"}
+
+	for _, wire := range [][]api.AgentVaultSubstitution{{short, long}, {long, short}} {
+		req := requestTo(t, "GET", "/")
+		req.Header.Set("X-A", "__TOKEN__")
+		req.Header.Set("X-B", "__TOKEN__V2")
+		applySubstitutions(req, "svc", toSubstitutions(wire))
+
+		if got := req.Header.Get("X-A"); got != "SECRET_A" {
+			t.Errorf("server order %q: X-A = %q, want SECRET_A", wire[0].Placeholder, got)
+		}
+		if got := req.Header.Get("X-B"); got != "SECRET_B" {
+			t.Errorf("server order %q: X-B = %q, want SECRET_B", wire[0].Placeholder, got)
+		}
+	}
+}
+
 func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
 	for _, tc := range []struct{ name, target, wantURI string }{
 		{

From 0d261a15024dff120291a4aabd259c4fcb527993 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Thu, 17 Sep 2026 16:59:54 +0530
Subject: [PATCH 13/25] fix(agent-vault): match an encoded placeholder in the
 query too

A client that builds the query from parameters rather than a string
percent-encodes first, so {{TOKEN}} arrives as %7B%7BTOKEN%7D%7D and the
literal match missed it. The placeholder then reached the third party and
the 401 that came back explained nothing.

The path surface already falls back to the escaped form; this is the same
fallback with query escaping. Underscore-style placeholders are never
encoded and were never affected.
---
 packages/agentvault/rewrite.go                | 18 ++++++++++++----
 .../rewrite_transformations_test.go           | 21 +++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 68629416..db140841 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -95,10 +95,20 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 
 		// Escaped, because RawQuery goes on the wire verbatim. A base64 key containing '+' would otherwise
 		// arrive as a space, and one containing '&' would split into a second parameter.
-		if sub.surfaces[surfaceQuery] && strings.Contains(req.URL.RawQuery, sub.placeholder) {
-			if v, ok := replaceWithinLimit(req.URL.RawQuery, sub.placeholder, url.QueryEscape(real), maxBodyRewriteSize); ok {
-				req.URL.RawQuery = v
-				changed[surfaceQuery] = true
+		if sub.surfaces[surfaceQuery] {
+			// A client that builds the query from parameters rather than a string percent-encodes the
+			// placeholder first, so `{{TOKEN}}` arrives as `%7B%7BTOKEN%7D%7D`. The path surface already
+			// falls back this way; without it the placeholder reaches the third party and the 401 that
+			// comes back says nothing about why.
+			needle := sub.placeholder
+			if !strings.Contains(req.URL.RawQuery, needle) {
+				needle = url.QueryEscape(sub.placeholder)
+			}
+			if strings.Contains(req.URL.RawQuery, needle) {
+				if v, ok := replaceWithinLimit(req.URL.RawQuery, needle, url.QueryEscape(real), maxBodyRewriteSize); ok {
+					req.URL.RawQuery = v
+					changed[surfaceQuery] = true
+				}
 			}
 		}
 
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 091228d0..afb75117 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -253,6 +253,27 @@ func TestAPlaceholderPrefixingAnotherStillSendsItsOwnSecret(t *testing.T) {
 	}
 }
 
+// A client building the query from parameters percent-encodes the placeholder first, so both forms have to
+// be matched. Underscore-style placeholders are never encoded and stand as the control.
+func TestAQuerySubstitutionMatchesTheEncodedPlaceholderToo(t *testing.T) {
+	cases := []struct {
+		placeholder string
+		wire        string
+	}{
+		{"__PAT__", "__PAT__"},
+		{"{{PAT}}", "{{PAT}}"},
+		{"{{PAT}}", "%7B%7BPAT%7D%7D"},
+	}
+
+	for _, c := range cases {
+		req := requestTo(t, "GET", "/v1?key="+c.wire)
+		applySubstitutions(req, "svc", []substitution{subOn(c.placeholder, "SECRET", surfaceQuery)})
+		if got := req.URL.RawQuery; got != "key=SECRET" {
+			t.Errorf("placeholder %q sent as %q: query = %q, want key=SECRET", c.placeholder, c.wire, got)
+		}
+	}
+}
+
 func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
 	for _, tc := range []struct{ name, target, wantURI string }{
 		{

From 11414a458f0ed9c93065104771083e70d28f5f83 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 01:28:03 +0530
Subject: [PATCH 14/25] fix(agent-vault): refuse a segment that lands as
 traversal once trailing space is dropped

hasUnsafeEscape refuses control bytes with "< 0x20", and a space is exactly
0x20, so "%20" walked past it. Windows and IIS drop a trailing space or dot
from a segment, so "/repos/..%20/admin" reads here as an ordinary segment
named "..%20" and arrives there as "..". A service pinned to /repos reached
/admin with the credential attached. "...." needed no encoding at all, since
the segment check only matched "." and ".." exactly.

Judged on the decoded segment rather than by widening the byte rule. Refusing
every 0x20 would have taken "/repos/my%20repo" with it, which is a real path
and is already pinned as allowed.
---
 packages/agentvault/policy.go      | 40 +++++++++++++++++++++++++++++-
 packages/agentvault/policy_test.go |  9 ++++++-
 2 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index eade5b77..e8dc9c4d 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -100,7 +100,7 @@ func isAmbiguousPath(escaped string) bool {
 		return true
 	}
 	for _, segment := range strings.Split(escaped, "/") {
-		if segment == "." || segment == ".." {
+		if isDotSegment(decodeBenignEscapes(segment)) {
 			return true
 		}
 	}
@@ -145,6 +145,44 @@ func hasUnsafeEscape(escaped string) bool {
 	return sawEscape && !utf8.Valid(decoded)
 }
 
+// Runs after hasUnsafeEscape, so every escape still standing decodes to something harmless. Only the
+// decoded form tells us whether a segment is all dots and spaces: "..%20" is not, ".. " is.
+func decodeBenignEscapes(segment string) string {
+	if !strings.Contains(segment, "%") {
+		return segment
+	}
+	out := make([]byte, 0, len(segment))
+	for i := 0; i < len(segment); i++ {
+		if segment[i] != '%' || i+2 >= len(segment) {
+			out = append(out, segment[i])
+			continue
+		}
+		hi, hiOk := unhex(segment[i+1])
+		lo, loOk := unhex(segment[i+2])
+		if !hiOk || !loOk {
+			out = append(out, segment[i])
+			continue
+		}
+		out = append(out, hi<<4|lo)
+		i += 2
+	}
+	return string(out)
+}
+
+// Windows and IIS strip trailing dots and spaces from a segment, so anything built only from those reads
+// as "." or ".." once it lands.
+func isDotSegment(segment string) bool {
+	if segment == "" {
+		return false
+	}
+	for i := 0; i < len(segment); i++ {
+		if segment[i] != '.' && segment[i] != ' ' {
+			return false
+		}
+	}
+	return true
+}
+
 func unhex(c byte) (byte, bool) {
 	switch {
 	case c >= '0' && c <= '9':
diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
index 277961a0..736c6298 100644
--- a/packages/agentvault/policy_test.go
+++ b/packages/agentvault/policy_test.go
@@ -61,7 +61,9 @@ func TestMethodPolicy(t *testing.T) {
 func TestPathPolicy(t *testing.T) {
 	svc := serviceWithPolicy(nil, []string{"/repos"})
 
-	allowed := []string{"/repos", "/repos/", "/repos/octo/hello", "/repos/a%20b"}
+	allowed := []string{
+		"/repos/my%20repo",
+		"/repos/...name", "/repos", "/repos/", "/repos/octo/hello", "/repos/a%20b"}
 	for _, path := range allowed {
 		t.Run("allows "+path, func(t *testing.T) {
 			if err := checkServicePolicy(svc, requestTo(t, "GET", path)); err != nil {
@@ -86,6 +88,11 @@ func TestPathPolicy(t *testing.T) {
 		"/repos;x/y",
 		"/repos/%2fadmin",
 		"/repos%5cx",
+		// Windows and IIS drop a trailing space or dot, so each of these lands as ".." upstream.
+		"/repos/..%20/admin",
+		"/repos/.. /admin",
+		"/repos/..../admin",
+		"/repos/.%20./admin",
 	}
 	for _, path := range blocked {
 		t.Run("blocks "+path, func(t *testing.T) {

From c305804cebf5165881c1e8171a6b6bdaf6c489d4 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 01:53:36 +0530
Subject: [PATCH 15/25] fix(agent-vault): refuse an echoing method whatever its
 case, and say so in the log

TRACE and TRACK make the upstream reflect the injected credential back in the
response body, so both are refused. The comparison was exact while allowsMethod
had always upper-cased, and Go passes the method through untouched, so a
lowercase "trace" walked past. The policy check caught it wherever a service
restricted methods; the gap was services restricting none, which is the default.

The refusal also returned straight out of the handler, ahead of the logging the
other refusals go through, leaving the one refusal that means somebody reached
for the credential with no record. Raised inside forward now, so it reads as
errPolicyBlocked like the rest: 403 rather than 405, and logged.
---
 packages/agentvault/proxy.go             | 13 +++++++------
 packages/agentvault/proxy_policy_test.go | 17 +++++++++++++++++
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 60672c41..5da6291b 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -346,12 +346,6 @@ func (ps *proxyServer) handlePlainForward(w http.ResponseWriter, r *http.Request
 }
 
 func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, scheme, hostname, port, sessionToken string) {
-	// TRACE and TRACK make the upstream reflect the injected credential back in the response body.
-	if r.Method == http.MethodTrace || r.Method == "TRACK" {
-		http.Error(w, "method not allowed", http.StatusMethodNotAllowed)
-		return
-	}
-
 	reqPath := r.URL.EscapedPath()
 	if len(reqPath) > maxLoggedPathLen {
 		reqPath = reqPath[:maxLoggedPathLen] + "...[truncated]"
@@ -458,6 +452,13 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 		return nil, nil, outcome, fmt.Errorf("%w: %w", errSessionResolve, err)
 	}
 
+	// TRACE and TRACK make the upstream reflect the injected credential back in the response body. Upper
+	// -cased like allowsMethod already was, or a lowercase "trace" walks past. Refused here rather than in
+	// the handler so it is logged like every other refusal.
+	if method := strings.ToUpper(req.Method); method == http.MethodTrace || method == "TRACK" {
+		return nil, nil, outcome, fmt.Errorf("method %s echoes headers back: %w", method, errPolicyBlocked)
+	}
+
 	matched := bestMatch(services, hostname, port)
 
 	if ps.blocksOffBundle(matched, hostname, port) {
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index ff0c1646..eac19d04 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -412,3 +412,20 @@ func TestTheLogSaysWhichSurfacesWereSubstituted(t *testing.T) {
 		}
 	})
 }
+
+func TestAnEchoingMethodIsRefusedWhateverItsCase(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, nil, nil, nil)
+	})
+
+	// Unrestricted on methods, so only the echo guard can refuse these. Go sends the method verbatim.
+	for _, method := range []string{"TRACE", "trace", "TRACK", "track"} {
+		status, body := do(t, client, method, fmt.Sprintf("https://%s/anything", host), "")
+		if status != http.StatusForbidden {
+			t.Fatalf("%s should be refused, got %d: %s", method, status, body)
+		}
+		if !strings.Contains(body, "echoes headers back") {
+			t.Fatalf("%s: unhelpful body %q", method, body)
+		}
+	}
+}

From 86bdc06742d3d658dcd3c1eb823d4ceed1cb8119 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 01:54:02 +0530
Subject: [PATCH 16/25] fix(agent-vault): strip the method-override headers
 where a service restricts methods

Rails, Laravel and Symfony treat X-HTTP-Method-Override and its two siblings as
the method to perform, so a service restricted to GET and POST was one header
away from a DELETE: the allowlist read POST off the wire, allowed it, attached
the credential, and the framework did the rest.

The path checks in this package already anticipate what an upstream does to a
request after it arrives, semicolons on Tomcat and backslashes on IIS among
them. The method check assumed the wire method was the one that runs.

Only where a service restricts methods. With no restriction the agent can send
DELETE outright, so the header buys it nothing and there is no control to
protect.
---
 packages/agentvault/policy.go            |  8 ++++++
 packages/agentvault/proxy.go             |  4 +++
 packages/agentvault/proxy_policy_test.go | 31 ++++++++++++++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index e8dc9c4d..453fdaba 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -31,6 +31,14 @@ func (s *resolvedService) allowsMethod(method string) bool {
 	return s.allowedMethods[strings.ToUpper(method)]
 }
 
+// Rails, Laravel and Symfony all honour these, so a POST carrying one performs the method it names. The
+// wire method is what the allowlist judged, so where there is an allowlist the header has to go.
+func stripMethodOverrideHeaders(header http.Header) {
+	for _, name := range []string{"X-HTTP-Method-Override", "X-Method-Override", "X-HTTP-Method"} {
+		header.Del(name)
+	}
+}
+
 func requestPath(req *http.Request) string {
 	path := req.URL.EscapedPath()
 	if path == "" {
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 5da6291b..12ecf5df 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -481,6 +481,10 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 	// Stripped before injecting, so a client's Connection header cannot delete the credential.
 	stripHopByHopHeaders(req.Header)
 
+	if matched != nil && matched.allowedMethods != nil {
+		stripMethodOverrideHeaders(req.Header)
+	}
+
 	if matched != nil {
 		// A credential is only ever injected over TLS, whatever port the pattern names.
 		if !strings.EqualFold(scheme, "https") {
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index eac19d04..b78365cf 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -429,3 +429,34 @@ func TestAnEchoingMethodIsRefusedWhateverItsCase(t *testing.T) {
 		}
 	}
 }
+
+func TestAMethodOverrideHeaderCannotOutrankTheAllowlist(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, []string{"GET", "POST"}, nil, nil, nil)
+	})
+
+	req, err := http.NewRequest("POST", fmt.Sprintf("https://%s/anything", host), strings.NewReader("x"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	req.Header.Set("X-HTTP-Method-Override", "DELETE")
+	req.Header.Set("X-Method-Override", "DELETE")
+	req.Header.Set("X-HTTP-Method", "DELETE")
+
+	resp, err := client.Do(req)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer resp.Body.Close()
+	raw, _ := io.ReadAll(resp.Body)
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("the POST itself is allowed, got %d: %s", resp.StatusCode, raw)
+	}
+
+	got := decodeEcho(t, string(raw))
+	for _, name := range []string{"X-Http-Method-Override", "X-Method-Override", "X-Http-Method"} {
+		if v := got.Headers[name]; len(v) > 0 {
+			t.Fatalf("%s reached the upstream as %v, so the allowlist can be outranked", name, v)
+		}
+	}
+}

From 071cd6f4171c46a0be9d19bea335f178a28b7f89 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 01:54:18 +0530
Subject: [PATCH 17/25] fix(agent-vault): three substitution defects review
 found

Encoded placeholders are matched whatever case their escapes use. Percent
-escapes carry no required case and clients differ, but the fallback built its
needle upper-cased and compared byte for byte, so `%7b%7bPAT%7d%7d` never
matched and no substitution happened. Both surfaces had it; the query fallback
landed yesterday carrying the same flaw. Normalising escape case costs nothing
in the branch that is about to rewrite the URL anyway, since substituting
invalidates any signature the agent computed itself.

A substitution that matches nothing now says so. The failure was a miss rather
than a leak, the placeholder going upstream and the agent seeing a third-party
401 it cannot explain, but it was silent, which this file avoids everywhere
else. Body substitutions are judged where they are applied and stay out of it.

Query values escape per RFC 3986 rather than as form data. QueryEscape turns a
space into '+', which anything reading the raw query takes literally, SigV4
signing among them. The path surface three lines up already had this right, and
the test could not have caught the difference, asserting through ParseQuery,
which turns '+' back into a space. It compares the wire query now.

The expansion limit compares by division. count*(len(new)-len(old)) overflows
int on the 386 and armv6 builds goreleaser ships and wraps negative, so the
guard answers "small enough" for a request that then fails to allocate. A 10MB
body of a five-character placeholder against a 1.2KB secret is enough, and the
agent picks the body while the secret comes from the service.
---
 packages/agentvault/rewrite.go                | 70 +++++++++++++++++--
 .../rewrite_transformations_test.go           | 49 +++++++++++--
 2 files changed, 110 insertions(+), 9 deletions(-)

diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index db140841..b5e2848b 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -69,6 +69,7 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 			continue
 		}
 		real := string(sub.value)
+		before := len(changed)
 
 		// Swapped in the escaped path so every other segment keeps the byte form the agent sent. Rewriting the
 		// decoded Path makes Go re-derive the wire path without re-escaping '/', and `group%2Fproject` would
@@ -79,7 +80,11 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 			// `{{TOKEN}}` reads here as `%7B%7BTOKEN%7D%7D` and matching only the typed form would miss it.
 			needle := sub.placeholder
 			if !strings.Contains(escaped, needle) {
+				// Percent-escapes carry no required case and clients differ, so the fallback matches against
+				// upper-cased escapes. Rewriting them is free here: substituting changes the URL anyway, so a
+				// request the agent signed itself could never have used this surface.
 				needle = escapedPathForm(sub.placeholder)
+				escaped = upperPercentEscapes(escaped)
 			}
 			if strings.Contains(escaped, needle) {
 				if v, ok := replaceWithinLimit(escaped, needle, url.PathEscape(real), maxBodyRewriteSize); ok {
@@ -100,12 +105,14 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 			// placeholder first, so `{{TOKEN}}` arrives as `%7B%7BTOKEN%7D%7D`. The path surface already
 			// falls back this way; without it the placeholder reaches the third party and the 401 that
 			// comes back says nothing about why.
+			rawQuery := req.URL.RawQuery
 			needle := sub.placeholder
-			if !strings.Contains(req.URL.RawQuery, needle) {
-				needle = url.QueryEscape(sub.placeholder)
+			if !strings.Contains(rawQuery, needle) {
+				needle = queryEscapedForm(sub.placeholder)
+				rawQuery = upperPercentEscapes(rawQuery)
 			}
-			if strings.Contains(req.URL.RawQuery, needle) {
-				if v, ok := replaceWithinLimit(req.URL.RawQuery, needle, url.QueryEscape(real), maxBodyRewriteSize); ok {
+			if strings.Contains(rawQuery, needle) {
+				if v, ok := replaceWithinLimit(rawQuery, needle, queryValueEscape(real), maxBodyRewriteSize); ok {
 					req.URL.RawQuery = v
 					changed[surfaceQuery] = true
 				}
@@ -125,6 +132,16 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 				}
 			}
 		}
+
+		// The body is rewritten after this loop, so a substitution that reaches it is judged there. Anything
+		// else that matched nothing sent its placeholder upstream, and the third party's 401 says nothing
+		// about why.
+		if !sub.surfaces[surfaceBody] && len(changed) == before {
+			log.Warn().
+				Str("service", serviceName).
+				Str("placeholder", sub.placeholder).
+				Msg("agent-vault: a substitution matched nothing in the request")
+		}
 	}
 
 	if bodySubstitutions(subs) && req.Body != nil {
@@ -144,6 +161,41 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 
 // The placeholder as EscapedPath would render it. The leading '/' keeps url.URL's `Path == "*"` case out of
 // it, and the encoder leaves a slash alone, so trimming it back off is exact.
+// QueryEscape is form encoding, where a space becomes '+'. Anything reading the raw query per RFC 3986,
+// SigV4 signing among them, takes that as a literal plus. Every other special is already %XX by then, so
+// the only '+' left to rewrite is a space.
+func queryValueEscape(value string) string {
+	return strings.ReplaceAll(url.QueryEscape(value), "+", "%20")
+}
+
+func queryEscapedForm(placeholder string) string {
+	return queryValueEscape(placeholder)
+}
+
+// Percent-escapes are case-insensitive, so matching is done against a copy with the hex digits upper-cased.
+// Same length as the input, so nothing else about the string moves.
+func upperPercentEscapes(s string) string {
+	if !strings.Contains(s, "%") {
+		return s
+	}
+	b := []byte(s)
+	for i := 0; i+2 < len(b); i++ {
+		if b[i] != '%' {
+			continue
+		}
+		b[i+1] = upperHexDigit(b[i+1])
+		b[i+2] = upperHexDigit(b[i+2])
+	}
+	return string(b)
+}
+
+func upperHexDigit(c byte) byte {
+	if c >= 'a' && c <= 'f' {
+		return c - 'a' + 'A'
+	}
+	return c
+}
+
 func escapedPathForm(placeholder string) string {
 	return strings.TrimPrefix((&url.URL{Path: "/" + placeholder}).EscapedPath(), "/")
 }
@@ -234,7 +286,15 @@ func replaceWithinLimit(s, old, replacement string, limit int) (string, bool) {
 	if count == 0 {
 		return s, true
 	}
-	if len(s)+count*(len(replacement)-len(old)) > limit {
+	delta := len(replacement) - len(old)
+	if delta > 0 {
+		// Division rather than count*delta, which overflows int on the 386 and armv6 builds and wraps to a
+		// negative, answering "small enough" for something that then fails to allocate.
+		room := limit - len(s)
+		if room < 0 || count > room/delta {
+			return s, false
+		}
+	} else if len(s)+count*delta > limit {
 		return s, false
 	}
 	return strings.ReplaceAll(s, old, replacement), true
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index afb75117..5bb57a97 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -319,10 +319,12 @@ func TestAPathSubstitutionLeavesTheRestOfThePathAlone(t *testing.T) {
 }
 
 func TestAQuerySubstitutionEscapesTheValue(t *testing.T) {
-	for _, tc := range []struct{ name, secret, wantKey string }{
-		{"a base64 key with a plus", "aB+cD/eF==", "aB+cD/eF=="},
-		{"a value with a space", "has space", "has space"},
-		{"a value with an ampersand", "a&page=99", "a&page=99"},
+	// wantRaw is asserted as well as wantKey: ParseQuery turns '+' back into a space, so a value escaped as
+	// form data rather than per RFC 3986 reads correctly here while going out wrong on the wire.
+	for _, tc := range []struct{ name, secret, wantKey, wantRaw string }{
+		{"a base64 key with a plus", "aB+cD/eF==", "aB+cD/eF==", "key=aB%2BcD%2FeF%3D%3D&page=2"},
+		{"a value with a space", "has space", "has space", "key=has%20space&page=2"},
+		{"a value with an ampersand", "a&page=99", "a&page=99", "key=a%26page%3D99&page=2"},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
 			req, _ := http.NewRequest("GET", "https://api.example.com/x?key=__PAT__&page=2", nil)
@@ -338,6 +340,45 @@ func TestAQuerySubstitutionEscapesTheValue(t *testing.T) {
 			if got := parsed.Get("page"); got != "2" {
 				t.Fatalf("the substitution disturbed another parameter: page=%q", got)
 			}
+			if req.URL.RawQuery != tc.wantRaw {
+				t.Fatalf("wire query = %q, want %q", req.URL.RawQuery, tc.wantRaw)
+			}
 		})
 	}
 }
+
+func TestAnEncodedPlaceholderMatchesWhateverCaseItsEscapesUse(t *testing.T) {
+	for _, tc := range []struct{ name, url, wantURI string }{
+		{"typed", "https://api.example.com/x/{{PAT}}", "/x/SECRET"},
+		{"upper escapes", "https://api.example.com/x/%7B%7BPAT%7D%7D", "/x/SECRET"},
+		{"lower escapes", "https://api.example.com/x/%7b%7bPAT%7d%7d", "/x/SECRET"},
+		{"query typed", "https://api.example.com/x?k={{PAT}}", "/x?k=SECRET"},
+		{"query upper", "https://api.example.com/x?k=%7B%7BPAT%7D%7D", "/x?k=SECRET"},
+		{"query lower", "https://api.example.com/x?k=%7b%7bPAT%7d%7d", "/x?k=SECRET"},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			surface := surfacePath
+			if strings.Contains(tc.url, "?") {
+				surface = surfaceQuery
+			}
+			req, _ := http.NewRequest("GET", tc.url, nil)
+			applySubstitutions(req, "svc", []substitution{subOn("{{PAT}}", "SECRET", surface)})
+			if got := req.URL.RequestURI(); got != tc.wantURI {
+				t.Fatalf("wire = %q, want %q", got, tc.wantURI)
+			}
+		})
+	}
+}
+
+func TestTheExpansionLimitHoldsWithoutOverflowing(t *testing.T) {
+	if _, ok := replaceWithinLimit("aaaa", "a", strings.Repeat("x", 10), 12); ok {
+		t.Fatal("an expansion past the limit should be refused")
+	}
+	if got, ok := replaceWithinLimit("ab", "a", "xy", 12); !ok || got != "xyb" {
+		t.Fatalf("an expansion within the limit should apply, got %q ok=%v", got, ok)
+	}
+	// Shrinking never needs the limit, and must not be refused by the division branch.
+	if got, ok := replaceWithinLimit("aaaa", "aa", "b", 12); !ok || got != "bb" {
+		t.Fatalf("a shrinking replacement should apply, got %q ok=%v", got, ok)
+	}
+}

From 8ef56d4bc680d481944c2173859d491cf82ad9f9 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 02:46:11 +0530
Subject: [PATCH 18/25] improvement(agent-vault): broker over plain http, not
 only https

The guard refused everything over http, not just the credential: a pass-through
service whose whole job is adding a header did nothing at all, and the log line
announced a refusal to attach a credential it never had. Both sibling products
inject regardless of scheme, agent-proxy with no check at all and the
open-source agent-vault through a purpose-built http path, so this was the only
one of the three that refused.

Refusing made nobody safer. The fallback it pushed an admin to is putting the
real credential in the agent's environment, where it sits permanently, which is
the thing the product exists to prevent. An API reachable only over http is
ordinary inside a network, and running it that way is the admin's call on their
own network.

A portless pattern still means 443, and that is now the whole of what keeps a
credential off a plaintext wire: injection is scheme-blind, so naming a port is
how a service opts in, and the config then says on its face that a credential
goes out unencrypted. What it does not defend against is an agent that opens a
tunnel to a plaintext port and speaks cleartext inside it, which is deliberate
and matches both siblings.

Nilling the match also stripped the service and access bundle from the request
log, so a plaintext request to a configured service was recorded as if nothing
matched it. The line carries the service again, and a brokered request over
http is tagged rather than warned about: every other warning in the package is
a failure, a refusal or a limit, never a config working as written.
---
 packages/agentvault/match.go                |   7 +-
 packages/agentvault/policy.go               |   1 -
 packages/agentvault/proxy.go                |  53 +++++-----
 packages/agentvault/proxy_plaintext_test.go | 111 ++++++++++++++++++++
 4 files changed, 139 insertions(+), 33 deletions(-)
 create mode 100644 packages/agentvault/proxy_plaintext_test.go

diff --git a/packages/agentvault/match.go b/packages/agentvault/match.go
index 6e5171cf..ab5eceff 100644
--- a/packages/agentvault/match.go
+++ b/packages/agentvault/match.go
@@ -5,8 +5,9 @@ import (
 	"strings"
 )
 
-// A pattern with no port covers every port in Agent Proxy's grammar, which lets plaintext port 80
-// through with the credential attached. Defaulting to 443 keeps that from happening here.
+// A pattern with no port covers every port in Agent Proxy's grammar, which lets plaintext port 80 through
+// with the credential attached. Defaulting to 443 keeps that from happening here, and is the whole of it:
+// injection itself is scheme-blind, so naming a plaintext port is how an admin opts a service into it.
 const defaultPort = "443"
 
 // hostPattern carries no path: paths are rejected at write time, since the matcher would compare the
@@ -15,7 +16,7 @@ type hostPattern struct {
 	host string
 	port string
 	// Whether the entry named a port itself. Only the exception list reads this: a service without one
-	// has to stay on 443 or a credential would go out in the clear, but an exception carries no
+	// stays on 443, since that is what keeps a credential off plaintext, but an exception carries no
 	// credential, so a bare host there means the host rather than one port of it.
 	portWritten bool
 }
diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index 453fdaba..3cb53cbb 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -10,7 +10,6 @@ import (
 
 var errPolicyBlocked = errors.New("blocked by service policy")
 
-// Runs before the plaintext refusal that nils a match, so a restriction holds on http:// too.
 func checkServicePolicy(svc *resolvedService, req *http.Request) error {
 	if !svc.allowsMethod(req.Method) {
 		return fmt.Errorf("service %q does not allow %s: %w", svc.name, req.Method, errPolicyBlocked)
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 12ecf5df..42d2b72c 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -247,7 +247,8 @@ func (ps *proxyServer) handleConnect(w http.ResponseWriter, r *http.Request) {
 	// Node's fetch tunnels http:// targets too and speaks plaintext inside, where every other client sends
 	// absolute-form. A tunnel is opaque bytes to an ordinary proxy, so that works everywhere else; here the
 	// first byte decides: a TLS record starts with 0x16, anything else is plain HTTP and takes the same path
-	// as absolute-form, which already refuses to attach a credential over plaintext.
+	// as absolute-form. The port stays the one the CONNECT line named, so a service still only matches if
+	// its pattern covers that port.
 	buffered := newBufferedConn(clientConn)
 	_ = clientConn.SetDeadline(time.Now().Add(tlsHandshakeTimeout))
 	first, err := buffered.reader.Peek(1)
@@ -291,8 +292,8 @@ func newBufferedConn(c net.Conn) *bufferedConn {
 
 func (b *bufferedConn) Read(p []byte) (int, error) { return b.reader.Read(p) }
 
-// The scheme is the tunnel's, not the inner request's: a credential is only ever attached on https, and
-// the target host comes from the CONNECT line so an agent cannot address one host through a tunnel to another.
+// The scheme is the tunnel's, not the inner request's, and the target host comes from the CONNECT line so
+// an agent cannot address one host through a tunnel to another.
 func (ps *proxyServer) serveTunnel(conn net.Conn, scheme, hostname, port, sessionToken string) {
 	listener := newOneShotListener(conn)
 	srv := &http.Server{
@@ -393,6 +394,9 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 	if matched != nil {
 		event = event.Str("service", matched.name).Str("accessBundle", matched.accessBundleName)
 	}
+	if outcome.brokered && !strings.EqualFold(scheme, "https") {
+		event = event.Bool("plaintext", true)
+	}
 	// The logged path is always the agent's, so without this a substitution that matched nothing reads
 	// exactly like one that fired.
 	if len(outcome.substituted) > 0 {
@@ -486,33 +490,24 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 	}
 
 	if matched != nil {
-		// A credential is only ever injected over TLS, whatever port the pattern names.
-		if !strings.EqualFold(scheme, "https") {
-			log.Warn().
-				Str("host", hostname).
-				Str("service", matched.name).
-				Msg("agent-vault: refusing to attach a credential over plaintext http")
-			matched = nil
-		} else {
-			// Substitutions first, so an injected real value can never itself be rewritten. The credential last,
-			// so a custom header naming the credential's own header loses rather than replacing the token.
-			outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
-			outcome.brokered = injectCustomHeaders(req, matched.customHeaders)
-			if injectCredential(req, &matched.credential) {
-				outcome.brokered = true
-			}
-			if len(outcome.substituted) > 0 {
-				outcome.brokered = true
-			}
+		// Substitutions first, so an injected real value can never itself be rewritten. The credential last,
+		// so a custom header naming the credential's own header loses rather than replacing the token.
+		outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
+		outcome.brokered = injectCustomHeaders(req, matched.customHeaders)
+		if injectCredential(req, &matched.credential) {
+			outcome.brokered = true
+		}
+		if len(outcome.substituted) > 0 {
+			outcome.brokered = true
+		}
 
-			if len(matched.allowedPathPrefixes) > 0 && containsSurface(outcome.substituted, surfacePath) {
-				if !pathAllowedAfterSubstitution(requestPath(req), req.URL.Path, matched.allowedPathPrefixes) {
-					// The path now carries the real credential, so it must not reach the body or the log.
-					return nil, matched, outcome, fmt.Errorf(
-						"service %q does not allow the path this request substitutes to: %w",
-						matched.name, errPolicyBlocked,
-					)
-				}
+		if len(matched.allowedPathPrefixes) > 0 && containsSurface(outcome.substituted, surfacePath) {
+			if !pathAllowedAfterSubstitution(requestPath(req), req.URL.Path, matched.allowedPathPrefixes) {
+				// The path now carries the real credential, so it must not reach the body or the log.
+				return nil, matched, outcome, fmt.Errorf(
+					"service %q does not allow the path this request substitutes to: %w",
+					matched.name, errPolicyBlocked,
+				)
 			}
 		}
 	}
diff --git a/packages/agentvault/proxy_plaintext_test.go b/packages/agentvault/proxy_plaintext_test.go
new file mode 100644
index 00000000..d9ee018a
--- /dev/null
+++ b/packages/agentvault/proxy_plaintext_test.go
@@ -0,0 +1,111 @@
+package agentvault
+
+import (
+	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+)
+
+// The leg an agent takes for an http:// upstream: absolute-form through the proxy, no CONNECT and no TLS
+// anywhere. A service reaches this fixture only by naming its port, which is what opts it into plaintext.
+func newPlaintextFixture(t *testing.T, build func(host string) *resolvedService) (*http.Client, string) {
+	t.Helper()
+
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		body, _ := io.ReadAll(r.Body)
+		w.Header().Set("Content-Type", "application/json")
+		_ = json.NewEncoder(w).Encode(echoed{
+			Method:  r.Method,
+			Path:    r.URL.EscapedPath(),
+			Query:   r.URL.RawQuery,
+			Headers: r.Header,
+			Body:    string(body),
+		})
+	}))
+	t.Cleanup(upstream.Close)
+
+	upstreamURL, err := url.Parse(upstream.URL)
+	if err != nil {
+		t.Fatal(err)
+	}
+	host := "127.0.0.1:" + upstreamURL.Port()
+
+	ps := &proxyServer{transport: newUpstreamTransport()}
+	ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost})
+	ps.cache = newSessionCache(fixedResolver{services: []*resolvedService{build(host)}}, ps.pollInterval)
+
+	front := httptest.NewServer(http.HandlerFunc(ps.dispatch))
+	t.Cleanup(front.Close)
+
+	proxyURL, _ := url.Parse(front.URL)
+	proxyURL.User = url.UserPassword(ProxyAuthUsername, "agv_tok")
+
+	return &http.Client{Transport: &http.Transport{Proxy: http.ProxyURL(proxyURL)}}, host
+}
+
+func TestEverythingAServiceCarriesIsAttachedOverPlainHTTP(t *testing.T) {
+	client, host := newPlaintextFixture(t, func(h string) *resolvedService {
+		svc := policyService(h, nil, nil,
+			[]customHeader{{name: "X-Tenant", value: []byte("acme")}},
+			[]substitution{{
+				placeholder: "{{PAT}}",
+				surfaces:    map[string]bool{surfacePath: true},
+				value:       []byte("ghp_real"),
+			}},
+		)
+		svc.credential = credential{
+			kind:         credentialBearer,
+			headerName:   "Authorization",
+			headerPrefix: "Bearer",
+			value:        []byte("tok_real"),
+		}
+		return svc
+	})
+
+	status, payload := do(t, client, http.MethodGet, "http://"+host+"/repos/{{PAT}}", "")
+	if status != http.StatusOK {
+		t.Fatalf("status = %d, want 200: %s", status, payload)
+	}
+	got := decodeEcho(t, payload)
+
+	if auth := got.Headers["Authorization"]; len(auth) != 1 || auth[0] != "Bearer tok_real" {
+		t.Errorf("Authorization = %v, want the real credential", auth)
+	}
+	if tenant := got.Headers["X-Tenant"]; len(tenant) != 1 || tenant[0] != "acme" {
+		t.Errorf("X-Tenant = %v, want the custom header", tenant)
+	}
+	if got.Path != "/repos/ghp_real" {
+		t.Errorf("path = %q, want the substitution applied", got.Path)
+	}
+}
+
+// A pass-through service carries no credential, so before this it was refused for a credential it never
+// had and its headers were dropped with it.
+func TestAPassThroughServiceStillAddsItsHeadersOverPlainHTTP(t *testing.T) {
+	client, host := newPlaintextFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, nil, []customHeader{{name: "X-Tenant", value: []byte("acme")}}, nil)
+	})
+
+	status, payload := do(t, client, http.MethodGet, "http://"+host+"/things", "")
+	if status != http.StatusOK {
+		t.Fatalf("status = %d, want 200: %s", status, payload)
+	}
+	if tenant := decodeEcho(t, payload).Headers["X-Tenant"]; len(tenant) != 1 || tenant[0] != "acme" {
+		t.Errorf("X-Tenant = %v, want the custom header", tenant)
+	}
+}
+
+// Restrictions are not loosened by the upstream being plaintext.
+func TestAServiceRestrictionStillHoldsOverPlainHTTP(t *testing.T) {
+	client, host := newPlaintextFixture(t, func(h string) *resolvedService {
+		return policyService(h, []string{"GET"}, nil, nil, nil)
+	})
+
+	status, _ := do(t, client, http.MethodDelete, "http://"+host+"/things", "")
+	if status != http.StatusForbidden {
+		t.Errorf("status = %d, want 403", status)
+	}
+}

From c2686326bb1979cd9569f5e2556fe219641b6bb6 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 03:51:38 +0530
Subject: [PATCH 19/25] fix(agent-vault): four defects the review found in the
 request path

EscapedPath rebuilds a path from its decoded form whenever RawPath is not valid
encoding, and one literal '{' is enough to trigger it. The rebuild turns '%2F'
into a real '/', so hasUnsafeEscape never saw the escape it exists to refuse:
'/repos/a%2Fb' was a 403 and '/repos/a%2Fb/{x}' was a 200 that reached the
upstream as '/repos/a/b/...', two segments where the agent sent one, with the
credential attached. Braces cannot simply be refused, since the placeholder
syntax is made of them, so the bytes Go objects to are escaped before anything
reads the path and RawPath stays valid.

The rewrite runs only where Go would have rebuilt anyway, which is what keeps
it from touching a path Go already accepts: '!', '(', ')', '*', '[' and ']' are
rejected by shouldEscape but accepted by validEncoded, so escaping them would
change the wire and break a byte-compared prefix. Within that narrower set the
output is Go's own rebuild, except that a percent-triple the agent wrote is
carried through rather than decoded. That difference is the fix, so the wire
does change for '%41' as much as it does for '%2F'; what does not change is the
brace class, which Go was already sending escaped. The predicate is derived
from the standard library rather than transcribed, because the table behind
encodePath is generated and a copy would drift.

A body that cannot be read whole is refused rather than forwarded short. The
old safety net left ContentLength disagreeing with the bytes so the transport
would refuse, but a chunked upload declares -1 and there is nothing to leave
wrong, so the upstream received a well-formed partial request carrying the
credential, could not tell it was partial, and answered 200 while the log said
we had refused to forward a truncated one. Only that read failure is an error;
a skipped body, an encoded one and an oversized one stay the no-op they were.
The path and headers are rewritten before the body is read, so the surfaces
that fired are returned alongside the error: the request is refused, but the
record still has to say the credential was written into it. It is the agent's
own upload that broke, so it earns a 400 rather than the upstream's 502.

An opaque request target is refused. 'http:admin/secrets' parses to an empty
path and a non-empty Opaque; the plain door already turned it away, though by
way of the empty host rather than the shape, and the tunnel reached forwardHTTP
directly, so with no path prefixes configured it went upstream as 'GET
admin/secrets HTTP/1.1' with a real credential. Not an access bypass, since a
service with no prefixes allows every path anyway, but the request was logged
with an empty path, so the one shape that looks deliberate was the one the
record could not show. The logged path comes from requestPath now, which cannot
be blank. The refusal sits above the path rewrite so an opaque target cannot
acquire a synthesized path on its way to being refused.

The post-substitution path check refuses what a substituted value can
introduce. It is weaker than isAmbiguousPath on purpose, because the value is
escaped by us and isAmbiguousPath refuses that very escaping, so a secret like
'org/repo' would be rejected on its own '%2F'. It judged only ';', '\' and dot
segments, which left '//', control bytes and an overlong '..' refused on
arrival and accepted once the real secret was in the path. Those three are
judged on the decoded form, where our own escaping is invisible. A secret
carrying an edge slash now fails that check rather than reaching the upstream
as a doubled separator, which is a behaviour change for a value a deployment
could hold.
---
 packages/agentvault/policy.go                 |  88 +++++++-
 packages/agentvault/policy_test.go            |  73 +++++++
 packages/agentvault/proxy.go                  |  25 ++-
 .../agentvault/proxy_requesttarget_test.go    | 197 ++++++++++++++++++
 packages/agentvault/rewrite.go                |  38 ++--
 .../rewrite_transformations_test.go           |  75 ++++++-
 6 files changed, 464 insertions(+), 32 deletions(-)
 create mode 100644 packages/agentvault/proxy_requesttarget_test.go

diff --git a/packages/agentvault/policy.go b/packages/agentvault/policy.go
index 3cb53cbb..e14ab32a 100644
--- a/packages/agentvault/policy.go
+++ b/packages/agentvault/policy.go
@@ -4,12 +4,17 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
 	"strings"
 	"unicode/utf8"
 )
 
 var errPolicyBlocked = errors.New("blocked by service policy")
 
+// The agent's own upload broke part way. Not a policy refusal and not an upstream failure, so it carries its
+// own status rather than landing in either of theirs.
+var errBodyUnreadable = errors.New("could not read the request body")
+
 func checkServicePolicy(svc *resolvedService, req *http.Request) error {
 	if !svc.allowsMethod(req.Method) {
 		return fmt.Errorf("service %q does not allow %s: %w", svc.name, req.Method, errPolicyBlocked)
@@ -38,10 +43,68 @@ func stripMethodOverrideHeaders(header http.Header) {
 	}
 }
 
+// Whether Go would escape a byte appearing unescaped in a path. Derived from the standard library rather
+// than transcribed from it: the table behind encodePath is generated, so a copy would be one Go release
+// away from disagreeing with the rebuild this guards against.
+var pathByteNeedsEscape = func() (table [256]bool) {
+	for b := 0; b < 256; b++ {
+		raw := string([]byte{byte(b)})
+		table[b] = (&url.URL{Path: raw}).EscapedPath() != raw
+	}
+	return table
+}()
+
+// EscapedPath falls back to rebuilding the path from its decoded form whenever RawPath is not valid
+// encoding, and one literal '{' is enough. The rebuild turns '%2F' into a real '/', so hasUnsafeEscape
+// never sees the escape it exists to refuse and the upstream receives a path the agent did not send.
+// Escaping those bytes ourselves keeps RawPath valid, so EscapedPath returns it untouched. The wire form is
+// unchanged either way: Go was already sending '%7B'.
+func normalizeRequestTarget(u *url.URL) {
+	if u.RawPath == "" || u.EscapedPath() == u.RawPath {
+		return
+	}
+	escaped := escapeInvalidPathBytes(u.RawPath)
+	decoded, err := url.PathUnescape(escaped)
+	if err != nil {
+		return
+	}
+	u.Path = decoded
+	u.RawPath = escaped
+}
+
+// A '%' opening a valid triple is carried through, so an escape the agent wrote is never escaped twice. A
+// malformed one cannot arrive: ParseRequestURI rejects it and the server answers 400 before the handler.
+func escapeInvalidPathBytes(raw string) string {
+	var out strings.Builder
+	out.Grow(len(raw))
+	for i := 0; i < len(raw); i++ {
+		c := raw[i]
+		if c == '%' && i+2 < len(raw) {
+			if _, hiOk := unhex(raw[i+1]); hiOk {
+				if _, loOk := unhex(raw[i+2]); loOk {
+					out.WriteString(raw[i : i+3])
+					i += 2
+					continue
+				}
+			}
+		}
+		if pathByteNeedsEscape[c] {
+			const hexDigits = "0123456789ABCDEF"
+			out.WriteByte('%')
+			out.WriteByte(hexDigits[c>>4])
+			out.WriteByte(hexDigits[c&0x0f])
+			continue
+		}
+		out.WriteByte(c)
+	}
+	return out.String()
+}
+
 func requestPath(req *http.Request) string {
 	path := req.URL.EscapedPath()
 	if path == "" {
-		// OPTIONS * arrives as "*" and is left alone; a genuinely empty path is the root.
+		// forwardHTTP refuses an opaque target before this runs, so the branch is a floor under that check
+		// rather than a shape expected here. A genuinely empty path is the root.
 		if req.URL.Opaque != "" {
 			return req.URL.Opaque
 		}
@@ -66,14 +129,29 @@ func pathAllowed(escaped string, prefixes []string) bool {
 	return matchesPrefix(escaped, prefixes)
 }
 
-// The path here is part-written by us: applySubstitutions escapes the value so it cannot add a segment, and
-// pathAllowed would refuse that very '%2F'. Only traversal can leave an allowed prefix, so only traversal is
-// refused, judged on the decoded path because that is what an upstream decoding '%2F' will route on. ';' and
-// '\' count as traversal here for the reason isAmbiguousPath gives.
+// Deliberately not isAmbiguousPath. The path here is part-written by us: applySubstitutions escapes the
+// value so it cannot add a segment, and isAmbiguousPath refuses that very '%2F', so a secret like
+// 'org/repo' would be rejected on its own escaping. Judged on the decoded path instead, which is both what
+// an upstream decoding '%2F' will route on and the form our own escaping is invisible in. Everything below
+// is a shape a substituted value could introduce; the agent's half of the path has already been through
+// isAmbiguousPath on arrival.
 func pathAllowedAfterSubstitution(escaped, decoded string, prefixes []string) bool {
 	if strings.ContainsAny(decoded, ";\\") {
 		return false
 	}
+	// Normalises differently per server, and an empty value substituted mid-path is how it arises here.
+	if strings.Contains(decoded, "//") {
+		return false
+	}
+	for i := 0; i < len(decoded); i++ {
+		if decoded[i] < 0x20 || decoded[i] == 0x7f {
+			return false
+		}
+	}
+	// '%c0%ae' is an overlong '.', which some servers read as a dot and route on.
+	if !utf8.ValidString(decoded) {
+		return false
+	}
 	for _, segment := range strings.Split(decoded, "/") {
 		if segment == "." || segment == ".." {
 			return false
diff --git a/packages/agentvault/policy_test.go b/packages/agentvault/policy_test.go
index 736c6298..1d1ef43b 100644
--- a/packages/agentvault/policy_test.go
+++ b/packages/agentvault/policy_test.go
@@ -3,6 +3,7 @@ package agentvault
 import (
 	"errors"
 	"net/http"
+	"net/url"
 	"strings"
 	"testing"
 )
@@ -248,3 +249,75 @@ func TestAnExplicitRootPrefixMatchesEverythingAnUnrestrictedServiceWould(t *test
 		})
 	}
 }
+
+// The post-substitution check is weaker than isAmbiguousPath on purpose, because we escape the value
+// ourselves and isAmbiguousPath refuses that escaping. It still has to refuse what a value can introduce.
+func TestThePostSubstitutionCheckRefusesWhatAValueCanIntroduce(t *testing.T) {
+	prefixes := toPathPrefixes([]string{"/repos"})
+	for _, tc := range []struct {
+		value string
+		want  bool
+		why   string
+	}{
+		{"ghp_plain", true, "an ordinary secret"},
+		{"org/repo", true, "a slash is escaped by us, not a separator"},
+		{"v1.2", true, "a dot inside a segment is not a dot segment"},
+		{"a%b", true, "a percent is escaped by us"},
+		{"", false, "an empty value leaves '//' behind"},
+		{"/admin", false, "a leading slash leaves '//' behind"},
+		{"admin/", false, "a trailing slash leaves '//' behind"},
+		{"a//b", false, "a doubled slash inside the value"},
+		{"a\tb", false, "a control byte"},
+		{"\xc0\xae\xc0\xae", false, "an overlong '..'"},
+	} {
+		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__PAT__/admin", nil)
+		if _, err := applySubstitutions(req, "github", []substitution{subOn("__PAT__", tc.value, surfacePath)}); err != nil {
+			t.Fatalf("%s: %v", tc.why, err)
+		}
+		if got := pathAllowedAfterSubstitution(requestPath(req), req.URL.Path, prefixes); got != tc.want {
+			t.Errorf("value %q (%s): allowed = %v, want %v (path %q)", tc.value, tc.why, got, tc.want, requestPath(req))
+		}
+	}
+}
+
+// Go rebuilds EscapedPath from the decoded path when RawPath is not valid encoding, which drops the very
+// '%2F' hasUnsafeEscape exists to refuse. Escaping those bytes first keeps the escape intact.
+func TestNormalizeRequestTargetKeepsAnEscapeGoWouldDrop(t *testing.T) {
+	for _, tc := range []struct{ raw, want, why string }{
+		{"/repos/a%2Fb", "/repos/a%2Fb", "already valid, left alone"},
+		{"/repos/a%2Fb/{x}", "/repos/a%2Fb/%7Bx%7D", "the brace is escaped, the %2F survives"},
+		{"/repos/{{PAT}}", "/repos/%7B%7BPAT%7D%7D", "a placeholder still reads as one"},
+		{"/repos/a|b", "/repos/a%7Cb", "a pipe"},
+		{"/repos/a%25b/{x}", "/repos/a%25b/%7Bx%7D", "an escaped percent is not escaped twice"},
+		{"/repos/plain", "/repos/plain", "nothing to do"},
+		// validEncoded accepts these, so Go never rebuilds and the guard leaves them exactly as sent. Escaping
+		// them would change the wire and break a byte-compared prefix.
+		{"/repos/a(b)!*[]'", "/repos/a(b)!*[]'", "sub-delims Go accepts unescaped"},
+		{"/repos/a%2Fb/c(d)", "/repos/a%2Fb/c(d)", "an escape plus sub-delims, still valid"},
+	} {
+		u, err := url.ParseRequestURI(tc.raw)
+		if err != nil {
+			t.Fatalf("%s: %v", tc.raw, err)
+		}
+		normalizeRequestTarget(u)
+		if got := u.EscapedPath(); got != tc.want {
+			t.Errorf("%s (%s): EscapedPath = %q, want %q", tc.raw, tc.why, got, tc.want)
+		}
+	}
+}
+
+// The escape that used to be dropped is the one the prefix check runs on, so a single brace decided whether
+// a path was refused.
+func TestAnEscapedSlashIsRefusedWhateverElseThePathCarries(t *testing.T) {
+	prefixes := toPathPrefixes([]string{"/repos"})
+	for _, raw := range []string{"/repos/a%2Fb", "/repos/a%2Fb/{x}"} {
+		u, err := url.ParseRequestURI(raw)
+		if err != nil {
+			t.Fatalf("%s: %v", raw, err)
+		}
+		normalizeRequestTarget(u)
+		if pathAllowed(u.EscapedPath(), prefixes) {
+			t.Errorf("%s was allowed; an escaped slash must be refused", raw)
+		}
+	}
+}
diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 42d2b72c..5b4fd305 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -347,11 +347,21 @@ func (ps *proxyServer) handlePlainForward(w http.ResponseWriter, r *http.Request
 }
 
 func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, scheme, hostname, port, sessionToken string) {
-	reqPath := r.URL.EscapedPath()
-	if len(reqPath) > maxLoggedPathLen {
-		reqPath = reqPath[:maxLoggedPathLen] + "...[truncated]"
+	// 'http:admin/secrets' parses to an empty path and a non-empty Opaque, which the upstream would receive
+	// as a request-target with no leading slash. handlePlainForward refuses the shape already; the tunnel
+	// reaches this handler directly, so the refusal belongs here where both doors meet.
+	if r.URL.Opaque != "" {
+		http.Error(w, "the request target must be a path; opaque forms are not forwarded", http.StatusBadRequest)
+		return
 	}
 
+	// Before anything reads the path: the policy check, the substitutions and the forward all have to see
+	// the bytes the agent sent, not the ones Go rebuilds.
+	normalizeRequestTarget(r.URL)
+
+	// requestPath rather than EscapedPath, so a brokered request is never recorded with a blank path.
+	reqPath := truncatePath(requestPath(r))
+
 	resp, matched, outcome, err := ps.forward(r, scheme, hostname, port, sessionToken)
 
 	// The body is fixed text per outcome, never err.Error(): an APIError carries the control-plane URL and
@@ -363,6 +373,9 @@ func (ps *proxyServer) forwardHTTP(w http.ResponseWriter, r *http.Request, schem
 	switch {
 	case errors.Is(err, errHostBlocked), errors.Is(err, errPolicyBlocked):
 		decision, status, body = decisionBlocked, http.StatusForbidden, err.Error()
+	case errors.Is(err, errBodyUnreadable):
+		// The agent's upload broke, so this is its request to retry rather than an upstream or policy failure.
+		decision, status, body = decisionBlocked, http.StatusBadRequest, err.Error()
 	case isProxyTokenRejected(err):
 		decision, status, body = decisionError, http.StatusServiceUnavailable, proxyRevokedBody
 	case isSessionGone(err):
@@ -492,7 +505,11 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 	if matched != nil {
 		// Substitutions first, so an injected real value can never itself be rewritten. The credential last,
 		// so a custom header naming the credential's own header loses rather than replacing the token.
-		outcome.substituted = applySubstitutions(req, matched.name, matched.substitutions)
+		substituted, err := applySubstitutions(req, matched.name, matched.substitutions)
+		outcome.substituted = substituted
+		if err != nil {
+			return nil, matched, outcome, err
+		}
 		outcome.brokered = injectCustomHeaders(req, matched.customHeaders)
 		if injectCredential(req, &matched.credential) {
 			outcome.brokered = true
diff --git a/packages/agentvault/proxy_requesttarget_test.go b/packages/agentvault/proxy_requesttarget_test.go
new file mode 100644
index 00000000..e7c6de1b
--- /dev/null
+++ b/packages/agentvault/proxy_requesttarget_test.go
@@ -0,0 +1,197 @@
+package agentvault
+
+import (
+	"bufio"
+	"crypto/tls"
+	"crypto/x509"
+	"encoding/json"
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+)
+
+// Literal bytes on the wire, because a Go client escapes a brace before sending and the whole point is what
+// an agent that does not can make the proxy do.
+func rawProxyRequest(t *testing.T, proxyHost, requestLine, hostHeader, tunnelTo string) (*http.Response, string) {
+	t.Helper()
+	conn, err := net.Dial("tcp", proxyHost)
+	if err != nil {
+		t.Fatal(err)
+	}
+	t.Cleanup(func() { _ = conn.Close() })
+	reader := bufio.NewReader(conn)
+
+	if tunnelTo != "" {
+		fmt.Fprintf(conn, "CONNECT %s HTTP/1.1\r\nHost: %s\r\nProxy-Authorization: Basic %s\r\n\r\n",
+			tunnelTo, tunnelTo, testProxyAuth)
+		established, err := http.ReadResponse(reader, nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if established.StatusCode != http.StatusOK {
+			t.Fatalf("CONNECT = %d", established.StatusCode)
+		}
+	}
+
+	fmt.Fprintf(conn, "%s\r\nHost: %s\r\nProxy-Authorization: Basic %s\r\nConnection: close\r\n\r\n",
+		requestLine, hostHeader, testProxyAuth)
+	resp, err := http.ReadResponse(reader, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	payload, _ := io.ReadAll(resp.Body)
+	_ = resp.Body.Close()
+	return resp, string(payload)
+}
+
+// base64("x-agent-vault:agv_tok")
+const testProxyAuth = "eC1hZ2VudC12YXVsdDphZ3ZfdG9r"
+
+func newRequestTargetFixture(t *testing.T, prefixes []string) (proxyHost, upstreamHost string) {
+	t.Helper()
+	upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_ = json.NewEncoder(w).Encode(echoed{Path: r.URL.EscapedPath()})
+	}))
+	t.Cleanup(upstream.Close)
+	uu, _ := url.Parse(upstream.URL)
+	upstreamHost = "127.0.0.1:" + uu.Port()
+
+	key, cert, err := generateRootCa()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ps := &proxyServer{transport: newUpstreamTransport(), ca: newCaManager(key, cert)}
+	ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost})
+	ps.cache = newSessionCache(fixedResolver{services: []*resolvedService{
+		policyService(upstreamHost, nil, prefixes, nil, []substitution{
+			{placeholder: "{{PAT}}", surfaces: map[string]bool{surfacePath: true}, value: []byte("ghp_real")},
+		}),
+	}}, ps.pollInterval)
+
+	front := httptest.NewServer(http.HandlerFunc(ps.dispatch))
+	t.Cleanup(front.Close)
+	fu, _ := url.Parse(front.URL)
+	return fu.Host, upstreamHost
+}
+
+// One literal brace used to make Go rebuild the path, dropping the '%2F' the prefix check refuses, and the
+// upstream received two segments where the agent sent one.
+func TestALiteralBraceNoLongerHidesAnEscapedSlash(t *testing.T) {
+	proxyHost, upstreamHost := newRequestTargetFixture(t, []string{"/repos"})
+
+	resp, _ := rawProxyRequest(t, proxyHost,
+		fmt.Sprintf("GET http://%s/repos/a%%2Fb/{x} HTTP/1.1", upstreamHost), upstreamHost, "")
+	if resp.StatusCode != http.StatusForbidden {
+		t.Errorf("status = %d, want 403", resp.StatusCode)
+	}
+}
+
+func TestAPlaceholderInThePathStillSubstitutes(t *testing.T) {
+	proxyHost, upstreamHost := newRequestTargetFixture(t, []string{"/repos"})
+
+	resp, payload := rawProxyRequest(t, proxyHost,
+		fmt.Sprintf("GET http://%s/repos/{{PAT}} HTTP/1.1", upstreamHost), upstreamHost, "")
+	if resp.StatusCode != http.StatusOK {
+		t.Fatalf("status = %d, want 200: %s", resp.StatusCode, payload)
+	}
+	var got echoed
+	if err := json.Unmarshal([]byte(payload), &got); err != nil {
+		t.Fatalf("upstream did not echo JSON (%v): %s", err, payload)
+	}
+	if got.Path != "/repos/ghp_real" {
+		t.Errorf("upstream saw %q, want the substituted path", got.Path)
+	}
+}
+
+// 'http:admin/secrets' parses to an empty path and a non-empty Opaque. handlePlainForward refuses the shape,
+// the tunnel reaches forwardHTTP directly, and the upstream would have received a target with no leading
+// slash and a real credential on it.
+func TestAnOpaqueRequestTargetIsRefusedInsideATunnel(t *testing.T) {
+	proxyHost, upstreamHost := newRequestTargetFixture(t, nil)
+
+	resp, _ := rawProxyRequest(t, proxyHost, "GET http:admin/secrets HTTP/1.1", upstreamHost, upstreamHost)
+	if resp.StatusCode != http.StatusBadRequest {
+		t.Errorf("status = %d, want 400", resp.StatusCode)
+	}
+}
+
+// The same two cases through a real CONNECT + TLS tunnel, which is how an agent actually arrives. The raw
+// dial is still required: a Go client escapes the brace before sending, which is the bug's blind spot.
+func TestTheRequestTargetHoldsThroughATLSTunnel(t *testing.T) {
+	upstream := httptest.NewTLSServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		_ = json.NewEncoder(w).Encode(echoed{Path: r.URL.EscapedPath()})
+	}))
+	defer upstream.Close()
+	uu, _ := url.Parse(upstream.URL)
+	upstreamHost := "127.0.0.1:" + uu.Port()
+
+	pool := x509.NewCertPool()
+	pool.AddCert(upstream.Certificate())
+	transport := newUpstreamTransport()
+	transport.TLSClientConfig = &tls.Config{RootCAs: pool}
+
+	key, cert, err := generateRootCa()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ps := &proxyServer{transport: transport, ca: newCaManager(key, cert)}
+	ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost})
+	ps.cache = newSessionCache(fixedResolver{services: []*resolvedService{
+		policyService(upstreamHost, nil, []string{"/repos"}, nil, []substitution{
+			{placeholder: "{{PAT}}", surfaces: map[string]bool{surfacePath: true}, value: []byte("ghp_real")},
+		}),
+	}}, ps.pollInterval)
+	front := httptest.NewServer(http.HandlerFunc(ps.dispatch))
+	defer front.Close()
+	fu, _ := url.Parse(front.URL)
+
+	clientPool := x509.NewCertPool()
+	clientPool.AddCert(cert)
+
+	send := func(target string) (int, string) {
+		conn, err := net.Dial("tcp", fu.Host)
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer conn.Close()
+		fmt.Fprintf(conn, "CONNECT %s HTTP/1.1\r\nHost: %s\r\nProxy-Authorization: Basic %s\r\n\r\n",
+			upstreamHost, upstreamHost, testProxyAuth)
+		reader := bufio.NewReader(conn)
+		if established, err := http.ReadResponse(reader, nil); err != nil || established.StatusCode != http.StatusOK {
+			t.Fatalf("CONNECT failed: %v", err)
+		}
+
+		tlsConn := tls.Client(conn, &tls.Config{RootCAs: clientPool, ServerName: "127.0.0.1"})
+		if err := tlsConn.Handshake(); err != nil {
+			t.Fatal(err)
+		}
+		fmt.Fprintf(tlsConn, "GET %s HTTP/1.1\r\nHost: %s\r\nConnection: close\r\n\r\n", target, upstreamHost)
+		resp, err := http.ReadResponse(bufio.NewReader(tlsConn), nil)
+		if err != nil {
+			t.Fatal(err)
+		}
+		payload, _ := io.ReadAll(resp.Body)
+		_ = resp.Body.Close()
+		return resp.StatusCode, string(payload)
+	}
+
+	if status, _ := send("/repos/a%2Fb/{x}"); status != http.StatusForbidden {
+		t.Errorf("an escaped slash beside a brace = %d, want 403", status)
+	}
+
+	status, payload := send("/repos/{{PAT}}")
+	if status != http.StatusOK {
+		t.Fatalf("placeholder = %d, want 200: %s", status, payload)
+	}
+	var got echoed
+	if err := json.Unmarshal([]byte(payload), &got); err != nil {
+		t.Fatalf("upstream did not echo JSON (%v): %s", err, payload)
+	}
+	if got.Path != "/repos/ghp_real" {
+		t.Errorf("upstream saw %q, want the substituted path", got.Path)
+	}
+}
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index b5e2848b..811992c2 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -62,7 +62,7 @@ func injectCustomHeaders(req *http.Request, customHeaders []customHeader) bool {
 
 // A body it cannot rewrite is logged rather than skipped in silence: the placeholder goes upstream and the
 // agent would otherwise see only a third-party 401.
-func applySubstitutions(req *http.Request, serviceName string, subs []substitution) []string {
+func applySubstitutions(req *http.Request, serviceName string, subs []substitution) ([]string, error) {
 	changed := map[string]bool{}
 	for _, sub := range subs {
 		if len(sub.placeholder) == 0 {
@@ -76,8 +76,8 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 		// arrive as two segments pointing at a different resource.
 		if sub.surfaces[surfacePath] {
 			escaped := req.URL.EscapedPath()
-			// EscapedPath re-encodes the whole path when what the agent sent is not already valid encoding, so
-			// `{{TOKEN}}` reads here as `%7B%7BTOKEN%7D%7D` and matching only the typed form would miss it.
+			// normalizeRequestTarget has already escaped whatever Go would have objected to, so a `{{TOKEN}}`
+			// on the wire reads here as `%7B%7BTOKEN%7D%7D` and matching only the typed form would miss it.
 			needle := sub.placeholder
 			if !strings.Contains(escaped, needle) {
 				// Percent-escapes carry no required case and clients differ, so the fallback matches against
@@ -144,10 +144,16 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 		}
 	}
 
+	// The path, query and header surfaces are already rewritten by now, so a body that cannot be read has to
+	// hand back what fired alongside the error. The request is refused, but the record still has to say the
+	// credential was written into it.
+	var bodyErr error
 	if bodySubstitutions(subs) && req.Body != nil {
-		if applyBodySubstitutions(req, serviceName, subs) {
+		replaced, err := applyBodySubstitutions(req, serviceName, subs)
+		if replaced {
 			changed[surfaceBody] = true
 		}
+		bodyErr = err
 	}
 
 	surfaces := make([]string, 0, len(changed))
@@ -156,7 +162,7 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 			surfaces = append(surfaces, surface)
 		}
 	}
-	return surfaces
+	return surfaces, bodyErr
 }
 
 // The placeholder as EscapedPath would render it. The leading '/' keeps url.URL's `Path == "*"` case out of
@@ -209,16 +215,16 @@ func bodySubstitutions(subs []substitution) bool {
 	return false
 }
 
-func applyBodySubstitutions(req *http.Request, serviceName string, subs []substitution) bool {
+func applyBodySubstitutions(req *http.Request, serviceName string, subs []substitution) (bool, error) {
 	if req.Body == http.NoBody || req.ContentLength == 0 {
-		return false
+		return false, nil
 	}
 	if req.Header.Get("Content-Encoding") != "" {
 		log.Warn().
 			Str("service", serviceName).
 			Bool("hasContentEncoding", true).
 			Msg("agent-vault: body substitution skipped on an encoded body; the placeholder is going upstream unchanged")
-		return false
+		return false, nil
 	}
 	// Judged before reading, so an oversize body costs no memory. The check below still has to stand on its
 	// own: a chunked request declares -1, and a declared length is a claim rather than a fact.
@@ -226,25 +232,25 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 		log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
 			Int64("declaredBytes", req.ContentLength).
 			Msg("agent-vault: body larger than the substitution limit; the placeholder is going upstream unchanged")
-		return false
+		return false, nil
 	}
 
 	body, err := io.ReadAll(io.LimitReader(req.Body, maxBodyRewriteSize+1))
 	if err != nil {
-		// ContentLength is deliberately left disagreeing with the bytes, so http.Transport refuses the request.
-		// Correcting it would hand the upstream a well-formed shorter request it cannot tell from a complete
-		// one, turning a broken upload into a partial write nobody can take back.
+		// Refused outright rather than forwarded short. This used to leave ContentLength disagreeing with the
+		// bytes so http.Transport would refuse, but a chunked upload declares -1 and there is nothing to leave
+		// wrong, so the upstream received a well-formed partial request with the credential on it and could not
+		// tell. A partial write is not something an agent can take back.
 		_ = req.Body.Close()
-		req.Body = io.NopCloser(bytes.NewReader(body))
 		log.Warn().Err(err).Str("service", serviceName).Int("bytesRead", len(body)).
 			Msg("agent-vault: could not read the whole request body for substitution; refusing to forward a truncated one")
-		return false
+		return false, errBodyUnreadable
 	}
 	if len(body) > maxBodyRewriteSize {
 		req.Body = io.NopCloser(io.MultiReader(bytes.NewReader(body), req.Body))
 		log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
 			Msg("agent-vault: body larger than the substitution limit; the placeholder is going upstream unchanged")
-		return false
+		return false, nil
 	}
 	_ = req.Body.Close()
 
@@ -276,7 +282,7 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 	}
 	req.ContentLength = int64(len(rewritten))
 	req.Header.Set("Content-Length", fmt.Sprintf("%d", len(rewritten)))
-	return replaced
+	return replaced, nil
 }
 
 // Returns the input unchanged when the expansion would exceed limit, so a short placeholder mapped to a long
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 5bb57a97..7077d8d9 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -70,7 +70,8 @@ func TestAnOversizedDeclaredBodyIsNeverRead(t *testing.T) {
 	req.Body = &unreadableBody{t: t}
 	req.ContentLength = maxBodyRewriteSize + 1
 
-	if applyBodySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)}) {
+	replaced, _ := applyBodySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)})
+	if replaced {
 		t.Fatal("reported a substitution on a body it should not have touched")
 	}
 }
@@ -99,10 +100,13 @@ func TestABrokenUploadIsNotForwardedTruncated(t *testing.T) {
 	req.ContentLength = int64(len(full))
 	req.Header.Set("Content-Length", fmt.Sprintf("%d", len(full)))
 
-	applyBodySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)})
+	_, err := applyBodySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)})
 
+	if !errors.Is(err, errBodyUnreadable) {
+		t.Fatalf("err = %v, want errBodyUnreadable; a partial body must not reach the upstream", err)
+	}
 	if req.ContentLength != int64(len(full)) {
-		t.Fatalf("ContentLength = %d, want the declared %d so the transport refuses", req.ContentLength, len(full))
+		t.Fatalf("ContentLength = %d, want the declared %d left alone", req.ContentLength, len(full))
 	}
 	if got := req.Header.Get("Content-Length"); got != fmt.Sprintf("%d", len(full)) {
 		t.Fatalf("Content-Length header = %q, want the declared length", got)
@@ -116,7 +120,7 @@ func TestABrokenUploadIsNotForwardedTruncated(t *testing.T) {
 func TestApplySubstitutions(t *testing.T) {
 	t.Run("path", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/repos/__TOKEN__/x", nil)
-		surfaces := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfacePath)})
+		surfaces, _ := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfacePath)})
 		if req.URL.Path != "/repos/real/x" {
 			t.Fatalf("path = %q", req.URL.Path)
 		}
@@ -127,7 +131,7 @@ func TestApplySubstitutions(t *testing.T) {
 
 	t.Run("path, placeholder Go re-encodes", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://gitlab.com/api/v4/projects/{{PROJECT}}/pipelines", nil)
-		surfaces := applySubstitutions(req, "gitlab", []substitution{subOn("{{PROJECT}}", "group/project", surfacePath)})
+		surfaces, _ := applySubstitutions(req, "gitlab", []substitution{subOn("{{PROJECT}}", "group/project", surfacePath)})
 		if len(surfaces) != 1 || surfaces[0] != surfacePath {
 			t.Fatalf("surfaces = %v", surfaces)
 		}
@@ -183,7 +187,7 @@ func TestApplySubstitutions(t *testing.T) {
 		body := `{"token":"__TOKEN__"}`
 		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
 		req.Header.Set("Content-Encoding", "gzip")
-		surfaces := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
+		surfaces, _ := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
 		got, _ := io.ReadAll(req.Body)
 		if string(got) != body {
 			t.Fatalf("body should be untouched, got %q", got)
@@ -206,7 +210,7 @@ func TestApplySubstitutions(t *testing.T) {
 	t.Run("a body with no placeholder in it is untouched", func(t *testing.T) {
 		body := `{"a":"b"}`
 		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
-		surfaces := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
+		surfaces, _ := applySubstitutions(req, "github", []substitution{subOn("__TOKEN__", "real", surfaceBody)})
 		got, _ := io.ReadAll(req.Body)
 		if string(got) != body {
 			t.Fatalf("body = %q", got)
@@ -382,3 +386,60 @@ func TestTheExpansionLimitHoldsWithoutOverflowing(t *testing.T) {
 		t.Fatalf("a shrinking replacement should apply, got %q ok=%v", got, ok)
 	}
 }
+
+// Reads a prefix, then fails, the way a dropped upload does.
+type truncatingBody struct {
+	head string
+	n    int
+}
+
+func (b *truncatingBody) Read(p []byte) (int, error) {
+	if b.n < len(b.head) {
+		n := copy(p, b.head[b.n:])
+		b.n += n
+		return n, nil
+	}
+	return 0, errors.New("connection reset mid-body")
+}
+
+func (b *truncatingBody) Close() error { return nil }
+
+// The safety net used to be "leave ContentLength disagreeing so http.Transport refuses". A chunked upload
+// declares -1, so there was nothing to leave wrong and the upstream received a partial request with the
+// credential on it, answering 200 to something the agent never finished sending.
+func TestABodyThatCannotBeReadWholeIsRefusedWhateverTheEncoding(t *testing.T) {
+	for _, tc := range []struct {
+		name          string
+		contentLength int64
+	}{
+		{"declared length", 1007},
+		{"chunked", -1},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			req, _ := http.NewRequest("POST", "https://api.github.com/x", nil)
+			req.Body = &truncatingBody{head: "only the first few bytes"}
+			req.ContentLength = tc.contentLength
+
+			_, err := applySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfaceBody)})
+			if !errors.Is(err, errBodyUnreadable) {
+				t.Fatalf("err = %v, want errBodyUnreadable", err)
+			}
+		})
+	}
+}
+
+// The path is rewritten before the body is read, so a refusal still has to record that the credential was
+// written into the request.
+func TestSurfacesAlreadySubstitutedSurviveABodyFailure(t *testing.T) {
+	req, _ := http.NewRequest("POST", "https://api.github.com/repos/__PAT__/x", nil)
+	req.Body = &truncatingBody{head: "only the first few bytes"}
+	req.ContentLength = -1
+
+	surfaces, err := applySubstitutions(req, "github", []substitution{subOn("__PAT__", "real", surfacePath, surfaceBody)})
+	if !errors.Is(err, errBodyUnreadable) {
+		t.Fatalf("err = %v, want errBodyUnreadable", err)
+	}
+	if len(surfaces) == 0 || surfaces[0] != surfacePath {
+		t.Errorf("surfaces = %v, want the path substitution recorded", surfaces)
+	}
+}

From 15f8d70b6c447072af29776238f0665e828d6ee1 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 22:19:42 +0530
Subject: [PATCH 20/25] fix(agent-vault): two the review found around the
 method rules and the body limit

The body rewrite sized its result with count times the growth, which is the
multiplication replaceWithinLimit was changed away from: on the 386 and armv6
builds it wraps and answers "small enough" for a body that then fails to
allocate. A one byte placeholder with an 8KB value is a legal service, so the
agent only has to send a body full of it. Division there too.

The override headers were stripped before the service's own headers went in, so
a service configured to send X-HTTP-Method-Override kept it, and the allowlist
it contradicts was the one that asked for the strip. Stripping after the
brokered headers makes it independent of who set the header.

The second has a test that fails without it. The first cannot: the arithmetic
only overflows where int is 32 bits, so on a 64-bit runner the check refuses the
body either way. The test pins the refusal, and both targets still cross-compile.
---
 packages/agentvault/proxy.go                   | 10 ++++++----
 packages/agentvault/proxy_policy_test.go       | 18 ++++++++++++++++++
 packages/agentvault/rewrite.go                 |  5 ++++-
 .../agentvault/rewrite_transformations_test.go | 16 ++++++++++++++++
 4 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 5b4fd305..768452c7 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -498,10 +498,6 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 	// Stripped before injecting, so a client's Connection header cannot delete the credential.
 	stripHopByHopHeaders(req.Header)
 
-	if matched != nil && matched.allowedMethods != nil {
-		stripMethodOverrideHeaders(req.Header)
-	}
-
 	if matched != nil {
 		// Substitutions first, so an injected real value can never itself be rewritten. The credential last,
 		// so a custom header naming the credential's own header loses rather than replacing the token.
@@ -514,6 +510,12 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 		if injectCredential(req, &matched.credential) {
 			outcome.brokered = true
 		}
+
+		// After the brokered headers rather than before them, so a custom header cannot reintroduce an
+		// override of the method the allowlist already judged.
+		if matched.allowedMethods != nil {
+			stripMethodOverrideHeaders(req.Header)
+		}
 		if len(outcome.substituted) > 0 {
 			outcome.brokered = true
 		}
diff --git a/packages/agentvault/proxy_policy_test.go b/packages/agentvault/proxy_policy_test.go
index b78365cf..a3691451 100644
--- a/packages/agentvault/proxy_policy_test.go
+++ b/packages/agentvault/proxy_policy_test.go
@@ -460,3 +460,21 @@ func TestAMethodOverrideHeaderCannotOutrankTheAllowlist(t *testing.T) {
 		}
 	}
 }
+
+func TestAnInjectedMethodOverrideCannotOutrankTheAllowlist(t *testing.T) {
+	client, host := newPolicyFixture(t, func(h string) *resolvedService {
+		return policyService(h, []string{"GET", "POST"}, nil, []customHeader{
+			{name: "X-HTTP-Method-Override", value: []byte("DELETE")},
+		}, nil)
+	})
+
+	status, body := do(t, client, "POST", fmt.Sprintf("https://%s/anything", host), "x")
+	if status != http.StatusOK {
+		t.Fatalf("the POST itself is allowed, got %d: %s", status, body)
+	}
+
+	got := decodeEcho(t, body)
+	if v := got.Headers["X-Http-Method-Override"]; len(v) > 0 {
+		t.Fatalf("the service's own header reached the upstream as %v, so the allowlist can be outranked", v)
+	}
+}
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 811992c2..e0a335a1 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -264,7 +264,10 @@ func applyBodySubstitutions(req *http.Request, serviceName string, subs []substi
 		if count == 0 {
 			continue
 		}
-		if len(rewritten)+count*(len(sub.value)-len(sub.placeholder)) > maxBodyRewriteSize {
+		// Division for the growing case, for the overflow reason replaceWithinLimit spells out below.
+		delta := len(sub.value) - len(sub.placeholder)
+		room := maxBodyRewriteSize - len(rewritten)
+		if delta > 0 && (room < 0 || count > room/delta) {
 			log.Warn().Str("service", serviceName).Int("limitBytes", maxBodyRewriteSize).
 				Msg("agent-vault: substituted body would exceed the limit; the placeholder is going upstream unchanged")
 			continue
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 7077d8d9..38d2f8c8 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -207,6 +207,22 @@ func TestApplySubstitutions(t *testing.T) {
 		}
 	})
 
+	t.Run("a body that only crosses the limit once substituted is forwarded untouched", func(t *testing.T) {
+		// Under the limit as sent, over it once every placeholder has grown. The count times the growth is
+		// what overflows int on a 32-bit build, so this is the case the division guards.
+		value := strings.Repeat("v", 8192)
+		body := strings.Repeat("__T__", 4096)
+		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))
+		surfaces, _ := applySubstitutions(req, "github", []substitution{subOn("__T__", value, surfaceBody)})
+		got, _ := io.ReadAll(req.Body)
+		if !bytes.Equal(got, []byte(body)) {
+			t.Fatalf("the body must go upstream unchanged (got %d bytes, want %d)", len(got), len(body))
+		}
+		if len(surfaces) != 0 {
+			t.Fatalf("nothing should be reported as changed, got %v", surfaces)
+		}
+	})
+
 	t.Run("a body with no placeholder in it is untouched", func(t *testing.T) {
 		body := `{"a":"b"}`
 		req, _ := http.NewRequest("POST", "https://api.github.com/x", strings.NewReader(body))

From 4aaa2fa06a334d0b4908242c7f382ec04bec0e26 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Fri, 18 Sep 2026 23:31:37 +0530
Subject: [PATCH 21/25] fix(agent-vault): refuse to broker plain HTTP to a
 default 443 port

A service that names no port defaults to 443, which everywhere else in the
product means TLS. Two shapes reach the proxy as plain HTTP on that port anyway:
a CONNECT tunnel that answers with plaintext instead of a handshake, and an
http:// URL that names 443. Neither is something a correct client produces, but
both matched such a service and had the credential attached and sent upstream in
the clear, where a TLS-only host rejects it after the token has already crossed
the network readable.

Refused now, before substitution, since a path substitution would otherwise put
the real value into a cleartext request. Only the 443 default is guarded: a
service that names a plaintext port is the admin opting in, and a non-default
port follows the agent because only it knows the upstream's protocol.

Both cases were unpinned; the two tests drive the raw tunnel and the absolute
form, and both attach the credential without the guard.
---
 packages/agentvault/proxy.go                  |  8 ++
 .../agentvault/proxy_plaintext_443_test.go    | 82 +++++++++++++++++++
 2 files changed, 90 insertions(+)
 create mode 100644 packages/agentvault/proxy_plaintext_443_test.go

diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 768452c7..055a7463 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -498,6 +498,14 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 	// Stripped before injecting, so a client's Connection header cannot delete the credential.
 	stripHopByHopHeaders(req.Header)
 
+	// A service reaches port 443 by default, which everywhere else in the product means TLS, so plain HTTP
+	// here is either a tunnel that declined to handshake or an http:// URL naming 443 — nothing legitimate.
+	// Refused before substitution, which would otherwise put a secret in the path of a cleartext request.
+	if matched != nil && scheme == "http" && port == "443" {
+		return nil, matched, outcome, fmt.Errorf(
+			"service %q expects TLS on port 443; refusing to broker plain HTTP: %w", matched.name, errPolicyBlocked)
+	}
+
 	if matched != nil {
 		// Substitutions first, so an injected real value can never itself be rewritten. The credential last,
 		// so a custom header naming the credential's own header loses rather than replacing the token.
diff --git a/packages/agentvault/proxy_plaintext_443_test.go b/packages/agentvault/proxy_plaintext_443_test.go
new file mode 100644
index 00000000..c4829cf5
--- /dev/null
+++ b/packages/agentvault/proxy_plaintext_443_test.go
@@ -0,0 +1,82 @@
+package agentvault
+
+import (
+	"bufio"
+	"fmt"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"strings"
+	"testing"
+)
+
+// A service that names no port defaults to 443, which everywhere else means TLS. Both cases below reach the
+// proxy as plain HTTP on that port: a CONNECT tunnel that never handshakes, and an http:// URL naming 443.
+// Neither is something a correct client produces, and before the guard both had the credential attached and
+// sent upstream in the clear. The refusal fires before any dial, so the fixture needs no live upstream.
+func newPlaintext443Fixture(t *testing.T) (proxyHost, serviceHost string) {
+	t.Helper()
+	serviceHost = "example.test"
+
+	key, cert, err := generateRootCa()
+	if err != nil {
+		t.Fatal(err)
+	}
+	ps := &proxyServer{transport: newUpstreamTransport(), ca: newCaManager(key, cert)}
+	ps.setConfig(ProxyConfig{TrafficPolicy: TrafficPolicyAnyHost})
+	ps.cache = newSessionCache(fixedResolver{services: []*resolvedService{
+		policyService(serviceHost, nil, nil, nil, []substitution{
+			{placeholder: "{{PAT}}", surfaces: map[string]bool{surfacePath: true}, value: []byte("ghp_real")},
+		}),
+	}}, ps.pollInterval)
+
+	front := httptest.NewServer(http.HandlerFunc(ps.dispatch))
+	t.Cleanup(front.Close)
+	fu, _ := url.Parse(front.URL)
+	return fu.Host, serviceHost
+}
+
+func TestPlainHTTPInsideA443TunnelIsRefused(t *testing.T) {
+	proxyHost, serviceHost := newPlaintext443Fixture(t)
+
+	conn, err := net.Dial("tcp", proxyHost)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer conn.Close()
+	reader := bufio.NewReader(conn)
+
+	fmt.Fprintf(conn, "CONNECT %s:443 HTTP/1.1\r\nHost: %s:443\r\nProxy-Authorization: Basic %s\r\n\r\n",
+		serviceHost, serviceHost, testProxyAuth)
+	established, err := http.ReadResponse(reader, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if established.StatusCode != http.StatusOK {
+		t.Fatalf("CONNECT = %d", established.StatusCode)
+	}
+
+	// Plain HTTP down the tunnel rather than a TLS handshake — the shape the guard exists for.
+	fmt.Fprintf(conn, "GET /repos/{{PAT}} HTTP/1.1\r\nHost: %s\r\nConnection: close\r\n\r\n", serviceHost)
+	resp, err := http.ReadResponse(reader, nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if resp.StatusCode != http.StatusForbidden {
+		t.Fatalf("status = %d, want 403", resp.StatusCode)
+	}
+}
+
+func TestAbsoluteFormHTTPToPort443IsRefused(t *testing.T) {
+	proxyHost, serviceHost := newPlaintext443Fixture(t)
+
+	resp, body := rawProxyRequest(t, proxyHost,
+		fmt.Sprintf("GET http://%s:443/repos/{{PAT}} HTTP/1.1", serviceHost), serviceHost+":443", "")
+	if resp.StatusCode != http.StatusForbidden {
+		t.Fatalf("status = %d, want 403: %s", resp.StatusCode, body)
+	}
+	if !strings.Contains(body, "expects TLS") {
+		t.Errorf("body %q does not name the reason", body)
+	}
+}

From 7b267391c7f5cbc278a064eb4f15505162d11586 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Sat, 19 Sep 2026 03:36:17 +0530
Subject: [PATCH 22/25] improvement(agent-vault): resolve a substitution
 placeholder inside a custom header value

A custom header value can now carry a placeholder, resolved from the service's
substitutions as it is injected, so one substitution can stand in for a secret
reused across several headers rather than storing it once per header.

Scoped to the value the admin configured. The agent's own request is rewritten
by the earlier pass and the credential is still attached untouched afterwards, so
nothing the agent sends reaches this resolution and the credential is never
rewritten by it.
---
 packages/agentvault/proxy.go                     |  2 +-
 packages/agentvault/rewrite.go                   | 13 ++++++++++++-
 .../agentvault/rewrite_transformations_test.go   | 16 +++++++++++++---
 3 files changed, 26 insertions(+), 5 deletions(-)

diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 055a7463..72a7f688 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -514,7 +514,7 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 		if err != nil {
 			return nil, matched, outcome, err
 		}
-		outcome.brokered = injectCustomHeaders(req, matched.customHeaders)
+		outcome.brokered = injectCustomHeaders(req, matched.customHeaders, matched.substitutions)
 		if injectCredential(req, &matched.credential) {
 			outcome.brokered = true
 		}
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index e0a335a1..5be88492 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -49,12 +49,23 @@ func injectCredential(req *http.Request, cred *credential) bool {
 
 // Written before the credential, so one colliding with the credential's header loses to it. Pass-through
 // injects nothing, which is why Authorization as a custom header on one still works.
-func injectCustomHeaders(req *http.Request, customHeaders []customHeader) bool {
+func injectCustomHeaders(req *http.Request, customHeaders []customHeader, subs []substitution) bool {
 	for _, header := range customHeaders {
 		value := string(header.value)
 		if header.prefix != "" {
 			value = header.prefix + " " + value
 		}
+		// A placeholder written into a header value is resolved here, so one substitution can stand for a
+		// secret reused across several headers. Scoped to the value the admin set, never the agent's request
+		// or the credential, so nothing the agent sends can steer it.
+		for _, sub := range subs {
+			if len(sub.placeholder) == 0 {
+				continue
+			}
+			if replaced, ok := replaceWithinLimit(value, sub.placeholder, string(sub.value), maxBodyRewriteSize); ok {
+				value = replaced
+			}
+		}
 		req.Header.Set(header.name, value)
 	}
 	return len(customHeaders) > 0
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 38d2f8c8..9b6b9dcd 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -27,7 +27,7 @@ func TestInjectCustomHeaders(t *testing.T) {
 		injectCustomHeaders(req, []customHeader{
 			{name: "X-Org-Id", prefix: "", value: []byte("acme")},
 			{name: "X-Api-Ver", prefix: "v", value: []byte("2")},
-		})
+		}, nil)
 		if got := req.Header.Get("X-Org-Id"); got != "acme" {
 			t.Fatalf("X-Org-Id = %q", got)
 		}
@@ -39,7 +39,7 @@ func TestInjectCustomHeaders(t *testing.T) {
 	t.Run("overwrites whatever the agent sent", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
 		req.Header.Set("X-Org-Id", "spoofed")
-		injectCustomHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
+		injectCustomHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}}, nil)
 		if got := req.Header.Get("X-Org-Id"); got != "acme" {
 			t.Fatalf("X-Org-Id = %q", got)
 		}
@@ -49,11 +49,21 @@ func TestInjectCustomHeaders(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
 		req.Header.Set("Connection", "X-Org-Id")
 		stripHopByHopHeaders(req.Header)
-		injectCustomHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}})
+		injectCustomHeaders(req, []customHeader{{name: "X-Org-Id", value: []byte("acme")}}, nil)
 		if got := req.Header.Get("X-Org-Id"); got != "acme" {
 			t.Fatalf("X-Org-Id = %q", got)
 		}
 	})
+
+	t.Run("a placeholder in the value is resolved from a substitution", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
+		injectCustomHeaders(req,
+			[]customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("__KEY__")}},
+			[]substitution{{placeholder: "__KEY__", value: []byte("real")}})
+		if got := req.Header.Get("Authorization"); got != "Bearer real" {
+			t.Fatalf("Authorization = %q, want the substitution resolved", got)
+		}
+	})
 }
 
 type unreadableBody struct{ t *testing.T }

From ba7005d2ad4e968bfe218afabf9997b52d2747bb Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Sat, 19 Sep 2026 03:41:50 +0530
Subject: [PATCH 23/25] test(agent-vault): drive the custom-header substitution
 through the proxy

The unit test exercised injectCustomHeaders alone. This sends a real request
through the proxy with a service whose custom header value carries a placeholder
and asserts the upstream received it resolved. It fails if the resolution is
removed, where the unit test's own call could not.
---
 packages/agentvault/proxy_plaintext_test.go | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/packages/agentvault/proxy_plaintext_test.go b/packages/agentvault/proxy_plaintext_test.go
index d9ee018a..e20a12ed 100644
--- a/packages/agentvault/proxy_plaintext_test.go
+++ b/packages/agentvault/proxy_plaintext_test.go
@@ -82,6 +82,25 @@ func TestEverythingAServiceCarriesIsAttachedOverPlainHTTP(t *testing.T) {
 	}
 }
 
+// A custom header value carrying a placeholder is resolved from the service's substitutions on the way out,
+// end to end through the proxy, so one secret can be referenced across headers.
+func TestACustomHeaderValueResolvesASubstitutionOverPlainHTTP(t *testing.T) {
+	client, host := newPlaintextFixture(t, func(h string) *resolvedService {
+		return policyService(h, nil, nil,
+			[]customHeader{{name: "X-Signature", prefix: "v1", value: []byte("__KEY__")}},
+			[]substitution{{placeholder: "__KEY__", value: []byte("s3cr3t")}},
+		)
+	})
+
+	status, payload := do(t, client, http.MethodGet, "http://"+host+"/things", "")
+	if status != http.StatusOK {
+		t.Fatalf("status = %d, want 200: %s", status, payload)
+	}
+	if sig := decodeEcho(t, payload).Headers["X-Signature"]; len(sig) != 1 || sig[0] != "v1 s3cr3t" {
+		t.Errorf("X-Signature = %v, want the placeholder resolved", sig)
+	}
+}
+
 // A pass-through service carries no credential, so before this it was refused for a credential it never
 // had and its headers were dropped with it.
 func TestAPassThroughServiceStillAddsItsHeadersOverPlainHTTP(t *testing.T) {

From 53b4fce9400ffcfc35d0a449f40c2a614f7cf8c7 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Sat, 19 Sep 2026 04:14:48 +0530
Subject: [PATCH 24/25] fix(agent-vault): gate the custom-header substitution
 on the header surface

A substitution ticked for the body or path alone was still resolved inside a
custom header value, since the loop checked only for a non-empty placeholder.
Ticking a surface is the admin saying the secret may appear there, so it now
resolves in a custom header only when the header surface is among them. A custom
header is a header, so the same tick governs both it and the agent's own headers.

A body-only substitution now leaves a colliding header value untouched, with a
test for it alongside the one that resolves on the header surface.
---
 packages/agentvault/proxy_plaintext_test.go         |  2 +-
 packages/agentvault/rewrite.go                      |  7 ++++---
 packages/agentvault/rewrite_transformations_test.go | 12 +++++++++++-
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/packages/agentvault/proxy_plaintext_test.go b/packages/agentvault/proxy_plaintext_test.go
index e20a12ed..c5f2eccf 100644
--- a/packages/agentvault/proxy_plaintext_test.go
+++ b/packages/agentvault/proxy_plaintext_test.go
@@ -88,7 +88,7 @@ func TestACustomHeaderValueResolvesASubstitutionOverPlainHTTP(t *testing.T) {
 	client, host := newPlaintextFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, nil,
 			[]customHeader{{name: "X-Signature", prefix: "v1", value: []byte("__KEY__")}},
-			[]substitution{{placeholder: "__KEY__", value: []byte("s3cr3t")}},
+			[]substitution{{placeholder: "__KEY__", surfaces: map[string]bool{surfaceHeader: true}, value: []byte("s3cr3t")}},
 		)
 	})
 
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 5be88492..57ebf230 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -56,10 +56,11 @@ func injectCustomHeaders(req *http.Request, customHeaders []customHeader, subs [
 			value = header.prefix + " " + value
 		}
 		// A placeholder written into a header value is resolved here, so one substitution can stand for a
-		// secret reused across several headers. Scoped to the value the admin set, never the agent's request
-		// or the credential, so nothing the agent sends can steer it.
+		// secret reused across several headers. Gated on the header surface: ticking it is the admin saying
+		// the secret may appear in a header, and a custom header is one. Never the agent's request or the
+		// credential, so nothing the agent sends can steer it.
 		for _, sub := range subs {
-			if len(sub.placeholder) == 0 {
+			if !sub.surfaces[surfaceHeader] || len(sub.placeholder) == 0 {
 				continue
 			}
 			if replaced, ok := replaceWithinLimit(value, sub.placeholder, string(sub.value), maxBodyRewriteSize); ok {
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 9b6b9dcd..9b29db7f 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -59,11 +59,21 @@ func TestInjectCustomHeaders(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
 		injectCustomHeaders(req,
 			[]customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("__KEY__")}},
-			[]substitution{{placeholder: "__KEY__", value: []byte("real")}})
+			[]substitution{{placeholder: "__KEY__", surfaces: map[string]bool{surfaceHeader: true}, value: []byte("real")}})
 		if got := req.Header.Get("Authorization"); got != "Bearer real" {
 			t.Fatalf("Authorization = %q, want the substitution resolved", got)
 		}
 	})
+
+	t.Run("a substitution not on the header surface leaves the value alone", func(t *testing.T) {
+		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
+		injectCustomHeaders(req,
+			[]customHeader{{name: "X-Sig", value: []byte("__KEY__")}},
+			[]substitution{{placeholder: "__KEY__", surfaces: map[string]bool{surfaceBody: true}, value: []byte("real")}})
+		if got := req.Header.Get("X-Sig"); got != "__KEY__" {
+			t.Fatalf("X-Sig = %q, want the body-only substitution left it untouched", got)
+		}
+	})
 }
 
 type unreadableBody struct{ t *testing.T }

From b9908bf7bb732b4051efab35784b8584f77e7fb5 Mon Sep 17 00:00:00 2001
From: saif <11242541+saifsmailbox98@users.noreply.github.com>
Date: Sat, 19 Sep 2026 05:03:45 +0530
Subject: [PATCH 25/25] fix(agent-vault): record a custom-header substitution
 and drop the false warning

Two problems on the same request. The audit line said nothing about a secret
resolved into a custom header, because injectCustomHeaders returned only whether
it wrote any header and never fed the substitution back. It now reports when it
resolved a placeholder, and the request record carries `header` in substituted
the way a request-surface substitution already does.

And applySubstitutions warned that a placeholder matched nothing whenever it did
not appear in the agent's request, which is every request for a placeholder that
lives only in an admin-set custom header, the feature's whole point. The warning
also blamed the service config for ordinary agent behaviour, an agent that
probes or omits the token, so it is dropped. The body-too-large and unreadable
logs stay, since those are failures the proxy actually hit.

Tests assert the audit line and the absent warning, which none did before.
---
 packages/agentvault/proxy.go                  |  6 +++++-
 packages/agentvault/proxy_plaintext_test.go   | 20 ++++++++++++++++++
 packages/agentvault/rewrite.go                | 21 +++++++------------
 .../rewrite_transformations_test.go           | 10 +++++++--
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/packages/agentvault/proxy.go b/packages/agentvault/proxy.go
index 72a7f688..81e68b5b 100644
--- a/packages/agentvault/proxy.go
+++ b/packages/agentvault/proxy.go
@@ -514,7 +514,11 @@ func (ps *proxyServer) forward(req *http.Request, scheme, hostname, port, sessio
 		if err != nil {
 			return nil, matched, outcome, err
 		}
-		outcome.brokered = injectCustomHeaders(req, matched.customHeaders, matched.substitutions)
+		brokered, resolvedInHeader := injectCustomHeaders(req, matched.customHeaders, matched.substitutions)
+		outcome.brokered = brokered
+		if resolvedInHeader && !containsSurface(outcome.substituted, surfaceHeader) {
+			outcome.substituted = append(outcome.substituted, surfaceHeader)
+		}
 		if injectCredential(req, &matched.credential) {
 			outcome.brokered = true
 		}
diff --git a/packages/agentvault/proxy_plaintext_test.go b/packages/agentvault/proxy_plaintext_test.go
index c5f2eccf..b8089d67 100644
--- a/packages/agentvault/proxy_plaintext_test.go
+++ b/packages/agentvault/proxy_plaintext_test.go
@@ -1,12 +1,17 @@
 package agentvault
 
 import (
+	"bytes"
 	"encoding/json"
 	"io"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
+
+	"github.com/rs/zerolog"
+	"github.com/rs/zerolog/log"
 )
 
 // The leg an agent takes for an http:// upstream: absolute-form through the proxy, no CONNECT and no TLS
@@ -85,6 +90,11 @@ func TestEverythingAServiceCarriesIsAttachedOverPlainHTTP(t *testing.T) {
 // A custom header value carrying a placeholder is resolved from the service's substitutions on the way out,
 // end to end through the proxy, so one secret can be referenced across headers.
 func TestACustomHeaderValueResolvesASubstitutionOverPlainHTTP(t *testing.T) {
+	var logs bytes.Buffer
+	restore := log.Logger
+	log.Logger = zerolog.New(&logs)
+	t.Cleanup(func() { log.Logger = restore })
+
 	client, host := newPlaintextFixture(t, func(h string) *resolvedService {
 		return policyService(h, nil, nil,
 			[]customHeader{{name: "X-Signature", prefix: "v1", value: []byte("__KEY__")}},
@@ -99,6 +109,16 @@ func TestACustomHeaderValueResolvesASubstitutionOverPlainHTTP(t *testing.T) {
 	if sig := decodeEcho(t, payload).Headers["X-Signature"]; len(sig) != 1 || sig[0] != "v1 s3cr3t" {
 		t.Errorf("X-Signature = %v, want the placeholder resolved", sig)
 	}
+
+	// The audit line has to record the injection, and the "matched nothing" warning must not fire on a
+	// placeholder that lives only in a custom header the agent never sends.
+	out := logs.String()
+	if !strings.Contains(out, `"substituted":["header"]`) {
+		t.Errorf("audit line did not record the header substitution: %s", out)
+	}
+	if strings.Contains(out, "matched nothing") {
+		t.Errorf("a resolved substitution was warned as unmatched: %s", out)
+	}
 }
 
 // A pass-through service carries no credential, so before this it was refused for a credential it never
diff --git a/packages/agentvault/rewrite.go b/packages/agentvault/rewrite.go
index 57ebf230..3ffbdb50 100644
--- a/packages/agentvault/rewrite.go
+++ b/packages/agentvault/rewrite.go
@@ -49,7 +49,9 @@ func injectCredential(req *http.Request, cred *credential) bool {
 
 // Written before the credential, so one colliding with the credential's header loses to it. Pass-through
 // injects nothing, which is why Authorization as a custom header on one still works.
-func injectCustomHeaders(req *http.Request, customHeaders []customHeader, subs []substitution) bool {
+// Returns whether any header was written, and whether a placeholder was resolved inside one, so the audit
+// record can report the injection the way a request-surface substitution is reported.
+func injectCustomHeaders(req *http.Request, customHeaders []customHeader, subs []substitution) (brokered, resolved bool) {
 	for _, header := range customHeaders {
 		value := string(header.value)
 		if header.prefix != "" {
@@ -63,13 +65,14 @@ func injectCustomHeaders(req *http.Request, customHeaders []customHeader, subs [
 			if !sub.surfaces[surfaceHeader] || len(sub.placeholder) == 0 {
 				continue
 			}
-			if replaced, ok := replaceWithinLimit(value, sub.placeholder, string(sub.value), maxBodyRewriteSize); ok {
-				value = replaced
+			if replacedValue, ok := replaceWithinLimit(value, sub.placeholder, string(sub.value), maxBodyRewriteSize); ok && replacedValue != value {
+				value = replacedValue
+				resolved = true
 			}
 		}
 		req.Header.Set(header.name, value)
 	}
-	return len(customHeaders) > 0
+	return len(customHeaders) > 0, resolved
 }
 
 // A body it cannot rewrite is logged rather than skipped in silence: the placeholder goes upstream and the
@@ -81,7 +84,6 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 			continue
 		}
 		real := string(sub.value)
-		before := len(changed)
 
 		// Swapped in the escaped path so every other segment keeps the byte form the agent sent. Rewriting the
 		// decoded Path makes Go re-derive the wire path without re-escaping '/', and `group%2Fproject` would
@@ -145,15 +147,6 @@ func applySubstitutions(req *http.Request, serviceName string, subs []substituti
 			}
 		}
 
-		// The body is rewritten after this loop, so a substitution that reaches it is judged there. Anything
-		// else that matched nothing sent its placeholder upstream, and the third party's 401 says nothing
-		// about why.
-		if !sub.surfaces[surfaceBody] && len(changed) == before {
-			log.Warn().
-				Str("service", serviceName).
-				Str("placeholder", sub.placeholder).
-				Msg("agent-vault: a substitution matched nothing in the request")
-		}
 	}
 
 	// The path, query and header surfaces are already rewritten by now, so a body that cannot be read has to
diff --git a/packages/agentvault/rewrite_transformations_test.go b/packages/agentvault/rewrite_transformations_test.go
index 9b29db7f..d259396b 100644
--- a/packages/agentvault/rewrite_transformations_test.go
+++ b/packages/agentvault/rewrite_transformations_test.go
@@ -57,22 +57,28 @@ func TestInjectCustomHeaders(t *testing.T) {
 
 	t.Run("a placeholder in the value is resolved from a substitution", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
-		injectCustomHeaders(req,
+		_, resolved := injectCustomHeaders(req,
 			[]customHeader{{name: "Authorization", prefix: "Bearer", value: []byte("__KEY__")}},
 			[]substitution{{placeholder: "__KEY__", surfaces: map[string]bool{surfaceHeader: true}, value: []byte("real")}})
 		if got := req.Header.Get("Authorization"); got != "Bearer real" {
 			t.Fatalf("Authorization = %q, want the substitution resolved", got)
 		}
+		if !resolved {
+			t.Fatal("resolved = false, want true so the audit line can record it")
+		}
 	})
 
 	t.Run("a substitution not on the header surface leaves the value alone", func(t *testing.T) {
 		req, _ := http.NewRequest("GET", "https://api.github.com/x", nil)
-		injectCustomHeaders(req,
+		_, resolved := injectCustomHeaders(req,
 			[]customHeader{{name: "X-Sig", value: []byte("__KEY__")}},
 			[]substitution{{placeholder: "__KEY__", surfaces: map[string]bool{surfaceBody: true}, value: []byte("real")}})
 		if got := req.Header.Get("X-Sig"); got != "__KEY__" {
 			t.Fatalf("X-Sig = %q, want the body-only substitution left it untouched", got)
 		}
+		if resolved {
+			t.Fatal("resolved = true, want false since nothing was substituted")
+		}
 	})
 }