feat: add non-terminal Headers rule

[TorstenDittmann]

What

Adds a rule type that carries response headers instead of deciding the request, so a consumer can attach headers (for example a security header on a path prefix) through the same rule and condition model as the other actions.

• Rules\Headers: takes conditions and a header map, with action headers.
• Rule::isTerminal(): whether a match ends evaluation. Defaults to true; Headers returns false.
• Firewall::verify(): a matching non-terminal rule is recorded and evaluation continues with the next rule.
• Firewall::getMatchedNonTerminalRules(): the non-terminal rules that matched during the last verify(), in evaluation order.

Behaviour

• Existing rules are unaffected: they are all terminal, and verify() and getLastMatchedRule() return what they did before. The change is additive.
• getLastMatchedRule() stays the terminal rule that decided the request. When only Headers rules match, it is null and verify() returns false, the same as when nothing matches.
• Evaluation still stops at the first terminal match, so a Headers rule placed after it is not evaluated.
• Header names must be valid HTTP tokens and values must not contain control characters (so a value cannot inject further headers). An empty header map is rejected. All three throw InvalidArgumentException, like Challenge does for an unknown type.

The library only collects the matches; writing the headers onto a response is left to the consumer.

Testing

composer test (60 tests), composer check and composer lint pass locally. New cases cover the rule itself, validation, continuing past a header match, collection order, rules after a terminal match, and the reset between verify() calls.

[greptile-apps]

Confidence Score: 4/5

[Medium risk] Adds a new non-terminal rule type for response headers.

The PR should not merge until header-name validation is fixed and the repository’s observable-behavior testing requirement is met.

Findings

1.  Trailing newline passes validation ▶
2.  Numeric header names rejected ▶
3.  Tests assert implementation flags ▶

Fix with agent prompt

### Issue 1
src/Rules/Headers.php:33
A name such as `"X-Test
"` passes this check because `$` can match just before a final newline. The rule then exposes a name that is not a valid HTTP token, so a consumer writing the collected headers may reject it or handle it unsafely. Anchor the match to the absolute end of the string.

```suggestion
            if (!\is_string($name) || preg_match('/^[A-Za-z0-9!#$%&\'*+.^_`|~-]+\z/', $name) !== 1) {
```

### Issue 2
src/Rules/Headers.php:33
A numeric-only name such as `"123"` is a valid HTTP token, but PHP converts it to an integer array key before this check. `is_string($name)` therefore rejects it, preventing callers from registering that valid header name.

### Issue 3
tests/RulesTest.php:81-82
These assertions check `isTerminal()` directly, while the firewall tests already show the observable behavior: evaluation continues after a Headers match. The repository requires tests of observable behavior rather than assertions that mirror implementation choices. Remove these flag checks before merging.

```suggestion

```

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

Adds a non-terminal Headers rule, collects matching rules during firewall evaluation, and documents consumer-side header application.

• Terminal rules continue to decide the request in order.
• Header-name validation needs correction, and one test checks an implementation flag rather than its behavior.

_{Reviews (1) · Last reviewed commit: "feat: add non-terminal Headers rule"}

[greptile-apps]

Trailing newline passes validation A name such as "X-Test " passes this check because $ can match just before a final newline. The rule then exposes a name that is not a valid HTTP token, so a consumer writing the collected headers may reject it or handle it unsafely. Anchor the match to the absolute end of the string.

Suggested change

	if (!\is_string($name) || preg_match('/^[A-Za-z0-9!#$%&\'*+.^_`|~-]+$/', $name) !== 1) {
	if (!\is_string($name) || preg_match('/^[A-Za-z0-9!#$%&\'*+.^_`|~-]+\z/', $name) !== 1) {

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/Rules/Headers.php
Line: 33

Comment:
**Trailing newline passes validation** A name such as `"X-Test
"` passes this check because `$` can match just before a final newline. The rule then exposes a name that is not a valid HTTP token, so a consumer writing the collected headers may reject it or handle it unsafely. Anchor the match to the absolute end of the string.

```suggestion
            if (!\is_string($name) || preg_match('/^[A-Za-z0-9!#$%&\'*+.^_`|~-]+\z/', $name) !== 1) {
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Numeric header names rejected A numeric-only name such as "123" is a valid HTTP token, but PHP converts it to an integer array key before this check. is_string($name) therefore rejects it, preventing callers from registering that valid header name.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/Rules/Headers.php
Line: 33

Comment:
**Numeric header names rejected** A numeric-only name such as `"123"` is a valid HTTP token, but PHP converts it to an integer array key before this check. `is_string($name)` therefore rejects it, preventing callers from registering that valid header name.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

[greptile-apps]

Tests assert implementation flags These assertions check isTerminal() directly, while the firewall tests already show the observable behavior: evaluation continues after a Headers match. The repository requires tests of observable behavior rather than assertions that mirror implementation choices. Remove these flag checks before merging.

Suggested change

	$this->assertFalse($rule->isTerminal());
	$this->assertTrue((new Deny())->isTerminal());

Context Used: Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. (source)

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/RulesTest.php
Line: 81-82

Comment:
**Tests assert implementation flags** These assertions check `isTerminal()` directly, while the firewall tests already show the observable behavior: evaluation continues after a Headers match. The repository requires tests of observable behavior rather than assertions that mirror implementation choices. Remove these flag checks before merging.

```suggestion

```

**Context Used:** Call out and harshly judge implementation-coupled tests. We don't mirror source code, configuration, or version pins in assertions. We test observable behavior; use linters for syntax and schema checks. ([source](https://app.greptile.com/review/custom-context?memory=instruction-0))

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!