[kjdev]

Summary by CodeRabbit

• Security

  • Fixed a JWT authentication bypass caused by validation relaxation state persisting across requests. Signature and expiration enforcement are now correctly tied to each request.

• Tests

  • Added a new nginx Test::Nginx script covering location-config persistence and ensuring both signature relaxation handling and built-in expiration checks behave correctly across multiple requests.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 255c77e2-5088-43fb-b1e1-33a07242eac7

📥 Commits

Reviewing files that changed from the base of the PR and between a079941 and 658e30b.

📒 Files selected for processing (1)

• CHANGELOG.md

✅ Files skipped from review due to trivial changes (1)

• CHANGELOG.md

---

📝 Walkthrough

Walkthrough

Fixes an authentication bypass where validate.sig and validate.exp flags in the shared worker loc_conf were mutated during JWT requirement evaluation and persisted across subsequent requests. The fix snapshots these flags into the request-scoped ngx_http_auth_jwt_ctx_t at validation start and routes all relaxation writes and enforcement checks through that per-request copy.

Changes

Authentication bypass fix: request-scoped validation flags

Layer / File(s)	Summary
Request-scoped validate struct and snapshot src/ngx_http_auth_jwt_module.c	ngx_http_auth_jwt_ctx_t gains validate fields; ngx_http_auth_jwt_validate() copies cf->validate.sig and cf->validate.exp into ctx->validate.* at the start of each request so loc_conf is never subsequently modified.
Requirement relaxation writes to ctx src/ngx_http_auth_jwt_module.c	In ngx_http_auth_jwt_validate_requirement(), satisfying an exp claim requirement clears ctx->validate.exp and satisfying an alg header requirement clears ctx->validate.sig, replacing the prior mutations of the shared cf->validate.* fields.
Enforcement checks gated on ctx flags src/ngx_http_auth_jwt_module.c	The exp enforcement and signature verification conditionals in ngx_http_auth_jwt_validate() are updated to read ctx->validate.exp and ctx->validate.sig instead of cf->validate.*.
Regression tests and changelog t/auth_jwt_require_loc_conf_persist.t, CHANGELOG.md	New Test::Nginx test confirms that alg=none relaxation does not bleed into subsequent requests and that per-request expiry checks reject expired tokens; CHANGELOG Security entry documents the fix.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐇 Hop, hop — the shared state's gone astray,
A snapshot per request saves the day!
loc_conf sits still, never to roam,
Each request carries flags of its own.
No bypass lingers, no token deceives —
The rabbit secured what the worker believes! 🔐

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix validate loc conf' is concise and directly references the core issue: treating JWT validation flags as request-scoped state instead of mutating shared location configuration, which aligns with the main security fix documented in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/validate-loc-conf

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

t/auth_jwt_require_loc_conf_persist.t (1)

42-70: ⚡ Quick win

Add an explicit exp-requirement persistence regression case.

This scenario validates builtin exp behavior, but it doesn’t exercise the historical mutation path where an exp requirement relaxed validation state. Add a case with auth_jwt_require_claim exp ... followed by a second request to confirm relaxation never leaks across requests.

Suggested test shape

+=== test: exp requirement relaxation does not persist across requests
+--- http_config
+include $TEST_NGINX_CONF_DIR/authorized_server.conf;
+map $http_x_token $jwt {
+  "match"   $test1_jwt;
+  "expired" $test1_invalid_exp_jwt;
+}
+--- config
+include $TEST_NGINX_CONF_DIR/jwt.conf;
+location / {
+  auth_jwt "" token=$jwt;
+  auth_jwt_key_file $TEST_NGINX_DATA_DIR/jwks.json;
+  auth_jwt_require_claim exp ge $jwt_nowtime;
+  include $TEST_NGINX_CONF_DIR/authorized_proxy.conf;
+}
+--- request eval
+[
+  "GET /",
+  "GET /"
+]
+--- more_headers eval
+[
+  "X-Token: match",
+  "X-Token: expired"
+]
+--- error_code eval
+[
+  200,
+  401
+]

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@t/auth_jwt_require_loc_conf_persist.t` around lines 42 - 70, Add a new test
case after the existing "builtin exp validation is applied on every request"
test that explicitly validates the `auth_jwt_require_claim exp` requirement
persistence across requests. The new test should include a location
configuration with `auth_jwt_require_claim exp` followed by multiple request
scenarios to confirm that any relaxation of validation state does not leak
between requests. Use the same pattern as the current test with request eval
arrays and corresponding error_code eval arrays to verify that the exp
validation behavior remains consistent and isolated per request.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@CHANGELOG.md`:
- Line 7: In the CHANGELOG.md entry, replace the spelling "builtin" with the
standard hyphenated form "built-in" to maintain consistency with user-facing
documentation conventions. The word appears in the context of describing the
"builtin exp / sig checks" in the security fix description and should be changed
to "built-in exp / sig checks".

---

Nitpick comments:
In `@t/auth_jwt_require_loc_conf_persist.t`:
- Around line 42-70: Add a new test case after the existing "builtin exp
validation is applied on every request" test that explicitly validates the
`auth_jwt_require_claim exp` requirement persistence across requests. The new
test should include a location configuration with `auth_jwt_require_claim exp`
followed by multiple request scenarios to confirm that any relaxation of
validation state does not leak between requests. Use the same pattern as the
current test with request eval arrays and corresponding error_code eval arrays
to verify that the exp validation behavior remains consistent and isolated per
request.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 08c00da9-33ac-4fd6-9e61-10b4346e893e

📥 Commits

Reviewing files that changed from the base of the PR and between 136c97b and a079941.

📒 Files selected for processing (3)

• CHANGELOG.md
• src/ngx_http_auth_jwt_module.c
• t/auth_jwt_require_loc_conf_persist.t