Merge c302da4249 into 1f049e5ebd

CHANGELOG.md

@@ -32,6 +32,7 @@
 ## Breaking Changes
 
 ## Changes since v7.15.2
+- [#3449](https://github.com/oauth2-proxy/oauth2-proxy/pull/3449) fix: preserve percent-encoded characters in upstream rewrite targets (#2105) (@mladjan-gadzic)
 
 - [#3477](https://github.com/oauth2-proxy/oauth2-proxy/pull/3477) chore(dep): bump go to 1.26 and migrate of reverse proxy handling
 

CLAUDE.md

@@ -0,0 +1,13 @@
+## Agent skills
+
+### Issue tracker
+
+Issues live as local markdown files under `.scratch/<feature>/`. See `docs/agents/issue-tracker.md`.
+
+### Triage labels
+
+Canonical role strings (`needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, `wontfix`). See `docs/agents/triage-labels.md`.
+
+### Domain docs
+
+Single-context: one `CONTEXT.md` + `docs/adr/` at the repo root. See `docs/agents/domain.md`.

docs/agents/domain.md

@@ -0,0 +1,35 @@
+# Domain Docs
+
+How the engineering skills should consume this repo's domain documentation when exploring the codebase.
+
+This repo is **single-context**: one `CONTEXT.md` + `docs/adr/` at the repo root.
+
+## Before exploring, read these
+
+- **`CONTEXT.md`** at the repo root
+- **`docs/adr/`** — read ADRs that touch the area you're about to work in
+
+If any of these files don't exist, **proceed silently**. Don't flag their absence; don't suggest creating them upfront. The producer skill (`/grill-with-docs`) creates them lazily when terms or decisions actually get resolved.
+
+## File structure
+
+```
+/
+├── CONTEXT.md
+├── docs/adr/
+│   ├── 0001-some-decision.md
+│   └── 0002-another-decision.md
+└── ...
+```
+
+## Use the glossary's vocabulary
+
+When your output names a domain concept (in an issue title, a refactor proposal, a hypothesis, a test name), use the term as defined in `CONTEXT.md`. Don't drift to synonyms the glossary explicitly avoids.
+
+If the concept you need isn't in the glossary yet, that's a signal — either you're inventing language the project doesn't use (reconsider) or there's a real gap (note it for `/grill-with-docs`).
+
+## Flag ADR conflicts
+
+If your output contradicts an existing ADR, surface it explicitly rather than silently overriding:
+
+> _Contradicts ADR-0007 (some decision) — but worth reopening because…_

docs/agents/issue-tracker.md

@@ -0,0 +1,19 @@
+# Issue tracker: Local Markdown
+
+Issues and PRDs for this repo live as markdown files in `.scratch/`.
+
+## Conventions
+
+- One feature per directory: `.scratch/<feature-slug>/`
+- The PRD is `.scratch/<feature-slug>/PRD.md`
+- Implementation issues are `.scratch/<feature-slug>/issues/<NN>-<slug>.md`, numbered from `01`
+- Triage state is recorded as a `Status:` line near the top of each issue file (see `triage-labels.md` for the role strings)
+- Comments and conversation history append to the bottom of the file under a `## Comments` heading
+
+## When a skill says "publish to the issue tracker"
+
+Create a new file under `.scratch/<feature-slug>/` (creating the directory if needed).
+
+## When a skill says "fetch the relevant ticket"
+
+Read the file at the referenced path. The user will normally pass the path or the issue number directly.

docs/agents/triage-labels.md

@@ -0,0 +1,17 @@
+# Triage Labels
+
+The skills speak in terms of five canonical triage roles. This file maps those roles to the actual label strings used in this repo's issue tracker.
+
+| Label in mattpocock/skills | Label in our tracker | Meaning                                  |
+| -------------------------- | -------------------- | ---------------------------------------- |
+| `needs-triage`             | `needs-triage`       | Maintainer needs to evaluate this issue  |
+| `needs-info`               | `needs-info`         | Waiting on reporter for more information |
+| `ready-for-agent`          | `ready-for-agent`    | Fully specified, ready for an AFK agent  |
+| `ready-for-human`          | `ready-for-human`    | Requires human implementation            |
+| `wontfix`                  | `wontfix`            | Will not be actioned                     |
+
+When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label string from this table.
+
+Since issues are local markdown files (see `issue-tracker.md`), record the label as the `Status:` line value near the top of the issue file.
+
+Edit the right-hand column to match whatever vocabulary you actually use.

pkg/upstream/rewrite.go

@@ -49,8 +49,20 @@ func rewritePath(rewriteRegExp *regexp.Regexp, rewriteTarget string, writer page
 			return
 		}
 
+		// When the original path was percent-encoded, RawPath holds the escaped
+		// form. Rewrite it too so the encoding is preserved when proxying to the
+		// upstream; otherwise EscapedPath() (via reqURL.String()) re-encodes the
+		// decoded Path and drops characters such as %2F and %3A. See issue #2105.
+		if reqURL.RawPath != "" {
+			newRawPath := rewriteRegExp.ReplaceAllString(reqURL.RawPath, rewriteTarget)
+			reqURL.RawPath = strings.SplitN(newRawPath, "?", 2)[0]
+		}
+
 		req.RequestURI = reqURL.String()
-		req.URL.Path = reqURL.Path // set path for websocket connections
+		// Set path and raw (encoded) path for websocket connections, which are
+		// proxied using req.URL rather than req.RequestURI.
+		req.URL.Path = reqURL.Path
+		req.URL.RawPath = reqURL.RawPath
 		next.ServeHTTP(rw, req)
 	})
 }

pkg/upstream/rewrite_test.go

@@ -63,5 +63,26 @@ var _ = Describe("Rewrite", func() {
 			expectedRequestURI: "http://example.com/article?id=blog-2021-01-01",
 			expectedURLPath:    "/article",
 		}),
+		Entry("when the path contains percent-encoded characters, the encoding is preserved", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile("^/app/prefix/(.*)$"),
+			rewriteTarget:      "/$1",
+			requestTarget:      "http://example.com/app/prefix/v1/id/data%3Aabc%2Fdef",
+			expectedRequestURI: "http://example.com/v1/id/data%3Aabc%2Fdef",
+			expectedURLPath:    "/v1/id/data:abc/def",
+		}),
+		Entry("when the encoded path is rewritten and an original query is preserved", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile("^/app/prefix/(.*)$"),
+			rewriteTarget:      "/$1",
+			requestTarget:      "http://example.com/app/prefix/data%2Fone?foo=bar",
+			expectedRequestURI: "http://example.com/data%2Fone?foo=bar",
+			expectedURLPath:    "/data/one",
+		}),
+		Entry("when the encoded path is matched by a non-anchored regexp", rewritePathTableInput{
+			rewriteRegex:       regexp.MustCompile("/prefix/(.*)"),
+			rewriteTarget:      "/$1",
+			requestTarget:      "http://example.com/app/prefix/data%2Fone",
+			expectedRequestURI: "http://example.com/app/data%2Fone",
+			expectedURLPath:    "/app/data/one",
+		}),
 	)
 })