diff --git a/CHANGELOG.md b/CHANGELOG.md index 788e82c2..1483816f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 00000000..84ef380e --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,13 @@ +## Agent skills + +### Issue tracker + +Issues live as local markdown files under `.scratch//`. 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`. diff --git a/docs/agents/domain.md b/docs/agents/domain.md new file mode 100644 index 00000000..c7aadbc1 --- /dev/null +++ b/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…_ diff --git a/docs/agents/issue-tracker.md b/docs/agents/issue-tracker.md new file mode 100644 index 00000000..a2f08fb0 --- /dev/null +++ b/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//` +- The PRD is `.scratch//PRD.md` +- Implementation issues are `.scratch//issues/-.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//` (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. diff --git a/docs/agents/triage-labels.md b/docs/agents/triage-labels.md new file mode 100644 index 00000000..b2e84fa4 --- /dev/null +++ b/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. diff --git a/pkg/upstream/rewrite.go b/pkg/upstream/rewrite.go index 343c740b..1f326dc8 100644 --- a/pkg/upstream/rewrite.go +++ b/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) }) } diff --git a/pkg/upstream/rewrite_test.go b/pkg/upstream/rewrite_test.go index 27a3ae42..98c2a23b 100644 --- a/pkg/upstream/rewrite_test.go +++ b/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", + }), ) })