You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@trafficserver.apache.org by "masaori335 (via GitHub)" <gi...@apache.org> on 2026/09/14 08:51:53 UTC
[PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
masaori335 opened a new pull request, #13675:
URL: https://github.com/apache/trafficserver/pull/13675
`modifiers: [NOT]` only caught an explicit `with NOT`, and `language: [in]` only caught the `[...]` value form, so `!expr`, `!=`, `!~`, `!in` and `in {10.0.0.0/8}` all compiled unchecked. AND and OR were already checked at `&&` and `||`; negation and IP-range membership are now consistent with that.
The check sits at the two sites where the source introduces negation, not at `_make_condition`, whose `negate` argument is also true for the `[NOT]` the compiler synthesises for a bare header test. Denying `NOT` must not reject `if inbound.req.X-Foo`; allowed-implicit-not pins that.
`modifiers` was undocumented, so the sandbox section gains it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4010536141
##########
tools/hrw4u/tests/data/sandbox/denied-modifier-not.input.txt:
##########
@@ -0,0 +1,13 @@
+REMAP {
+ if !internal() {
+ inbound.req.X-Not-Internal = "1";
+ }
+
+ if inbound.method != "GET" {
Review Comment:
Confirmed and fixed in 806fcf0. Mutation-checked: replacing the tuple with `(hrw4uParser.NEQ,)` passed before the fixture change and now fails it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#issuecomment-5673313599
[approve ci autest 1of4 2of4]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#issuecomment-5690667532
[approve ci autest 2of4]
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4010411524
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1029,6 +1029,9 @@ def visitComparison(self, ctx, *, last: bool = False) -> None:
else:
negate = operator.symbol.type in (hrw4uParser.NEQ, hrw4uParser.NOT_TILDE)
+ if negate and not self._sandbox_check(ctx, lambda: self._sandbox.check_modifier("NOT")):
+ return
Review Comment:
Returning on a denial is the existing contract for a denied construct: sections (:755),
VARS / SESSION_VARS (:835, :851), else (:986), elif (:993) and in over a
value set (:1057) all do it, and the admin guide says so for sections — "A denied section
causes the entire block to be rejected; the body is not validated."
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4010541176
##########
tools/hrw4u/tests/data/sandbox/denied-modifier-not.input.txt:
##########
@@ -0,0 +1,13 @@
+REMAP {
+ if !internal() {
+ inbound.req.X-Not-Internal = "1";
+ }
+
+ if inbound.method != "GET" {
Review Comment:
Fixed by 806fcf0a49f04c00191acfe4b0db26c91687165a
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4010536141
##########
tools/hrw4u/tests/data/sandbox/denied-modifier-not.input.txt:
##########
@@ -0,0 +1,13 @@
+REMAP {
+ if !internal() {
+ inbound.req.X-Not-Internal = "1";
+ }
+
+ if inbound.method != "GET" {
Review Comment:
Confirmed and fixed in 806fcf0. Mutation-checked: replacing the tuple with `(hrw4uParser.NEQ,)` passed before the fixture change and now fails it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4003699970
##########
doc/admin-guide/configuration/hrw4u.en.rst:
##########
@@ -778,9 +779,24 @@ Construct What it controls
``variables`` The entire ``VARS`` section and all variable usage
``else`` The ``else { ... }`` branch of conditionals
``elif`` The ``elif ... { ... }`` branch of conditionals
-``in`` The ``in [...]`` and ``!in [...]`` set membership operators
+``in`` Set membership: the ``[...]`` value form and
+ the ``{...}`` IP range form, negated or not
================ ===================================================
+Condition Modifiers
+-------------------
+
+The ``modifiers`` list accepts ``AND``, ``OR``, ``NOT``, ``NOCASE``, ``PRE``,
+``SUF``, ``EXT``, ``MID``, ``I``, ``L`` and ``QSA``. Entries match the modifier
+however it is written, not only the explicit ``with`` form: ``AND`` also covers
+``&&``, ``OR`` also covers ``||``, and ``NOT`` also covers ``!``, ``!=``,
+``!~`` and ``!in``.
+
+Negation that the compiler introduces on its own is not matched. A bare header
+test such as ``if inbound.req.X-Foo`` compiles to ``cond %{HEADER:X-Foo} =""
+[NOT]``, and denying ``NOT`` does not reject it — the policy governs what the
+source writes.
Review Comment:
The inline-literal markup appears unbalanced: `[NOT]`` has closing backticks but not an opening literal marker, which can break Sphinx rendering. Also, the example cond token (`%{HEADER:...}`) doesn’t match the emitted form shown in the new golden output (`%{CLIENT-HEADER:...}`); consider aligning the documentation example with actual compiler output to avoid confusion.
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1029,6 +1029,9 @@ def visitComparison(self, ctx, *, last: bool = False) -> None:
else:
negate = operator.symbol.type in (hrw4uParser.NEQ, hrw4uParser.NOT_TILDE)
+ if negate and not self._sandbox_check(ctx, lambda: self._sandbox.check_modifier("NOT")):
+ return
Review Comment:
These new early `return`s stop traversal/emission for the current node, which can suppress additional sandbox diagnostics inside the same expression subtree (e.g., `!(denied_fn() && ...)` would now report only the `NOT` denial and skip checking `denied_fn()`). Consider separating (1) validation/diagnostic collection from (2) code emission: record the policy violation but still visit relevant child nodes to collect other errors, while gating only the output generation.
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1051,6 +1054,8 @@ def visitComparison(self, ctx, *, last: bool = False) -> None:
cond_txt = f"{lhs} {regex_expr}"
case _ if ctx.iprange():
+ if not self._sandbox_check(ctx, lambda: self._sandbox.check_language("in")):
+ return
Review Comment:
These new early `return`s stop traversal/emission for the current node, which can suppress additional sandbox diagnostics inside the same expression subtree (e.g., `!(denied_fn() && ...)` would now report only the `NOT` denial and skip checking `denied_fn()`). Consider separating (1) validation/diagnostic collection from (2) code emission: record the policy violation but still visit relevant child nodes to collect other errors, while gating only the output generation.
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1148,6 +1153,8 @@ def emit_factor(self, ctx, *, last: bool = False) -> None:
match ctx:
case _ if ctx.getChildCount() == 2 and ctx.getChild(0).getText() == "!":
self._dbg("`NOT' detected")
+ if not self._sandbox_check(ctx, lambda: self._sandbox.check_modifier("NOT")):
+ return
Review Comment:
These new early `return`s stop traversal/emission for the current node, which can suppress additional sandbox diagnostics inside the same expression subtree (e.g., `!(denied_fn() && ...)` would now report only the `NOT` denial and skip checking `denied_fn()`). Consider separating (1) validation/diagnostic collection from (2) code emission: record the policy violation but still visit relevant child nodes to collect other errors, while gating only the output generation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4003845549
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1148,6 +1153,8 @@ def emit_factor(self, ctx, *, last: bool = False) -> None:
match ctx:
case _ if ctx.getChildCount() == 2 and ctx.getChild(0).getText() == "!":
self._dbg("`NOT' detected")
+ if not self._sandbox_check(ctx, lambda: self._sandbox.check_modifier("NOT")):
+ return
Review Comment:
Same as above
##########
tools/hrw4u/src/visitor.py:
##########
@@ -1051,6 +1054,8 @@ def visitComparison(self, ctx, *, last: bool = False) -> None:
cond_txt = f"{lhs} {regex_expr}"
case _ if ctx.iprange():
+ if not self._sandbox_check(ctx, lambda: self._sandbox.check_language("in")):
+ return
Review Comment:
Same as above
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4010440493
##########
tools/hrw4u/tests/data/sandbox/denied-modifier-not.input.txt:
##########
@@ -0,0 +1,13 @@
+REMAP {
+ if !internal() {
+ inbound.req.X-Not-Internal = "1";
+ }
+
+ if inbound.method != "GET" {
Review Comment:
The new denial fixture covers `!`, `!=`, and `!in`, but not `!~`. `!~` is a distinct lexer token and is handled by the separate `NOT_TILDE` branch in `negate`, so a regression that drops `NOT_TILDE` from that tuple would still pass this test suite. Please add a sandbox-denied `!~` comparison and update the expected error count.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "cmcfarlen (via GitHub)" <gi...@apache.org>.
cmcfarlen commented on PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#issuecomment-5796158888
Cherry-picked to the 10.2.x branch as 5a02191d6aaaae3019db0d3b96cbfe37d5be8591 for the 10.2.1 release.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 merged PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] hrw4u: enforce sandbox NOT and in on their implicit spellings [trafficserver]
Posted by "masaori335 (via GitHub)" <gi...@apache.org>.
masaori335 commented on code in PR #13675:
URL: https://github.com/apache/trafficserver/pull/13675#discussion_r4010414320
##########
doc/admin-guide/configuration/hrw4u.en.rst:
##########
@@ -778,9 +779,24 @@ Construct What it controls
``variables`` The entire ``VARS`` section and all variable usage
``else`` The ``else { ... }`` branch of conditionals
``elif`` The ``elif ... { ... }`` branch of conditionals
-``in`` The ``in [...]`` and ``!in [...]`` set membership operators
+``in`` Set membership: the ``[...]`` value form and
+ the ``{...}`` IP range form, negated or not
================ ===================================================
+Condition Modifiers
+-------------------
+
+The ``modifiers`` list accepts ``AND``, ``OR``, ``NOT``, ``NOCASE``, ``PRE``,
+``SUF``, ``EXT``, ``MID``, ``I``, ``L`` and ``QSA``. Entries match the modifier
+however it is written, not only the explicit ``with`` form: ``AND`` also covers
+``&&``, ``OR`` also covers ``||``, and ``NOT`` also covers ``!``, ``!=``,
+``!~`` and ``!in``.
+
+Negation that the compiler introduces on its own is not matched. A bare header
+test such as ``if inbound.req.X-Foo`` compiles to ``cond %{HEADER:X-Foo} =""
+[NOT]``, and denying ``NOT`` does not reject it — the policy governs what the
+source writes.
Review Comment:
Fixed by [9e08728](https://github.com/apache/trafficserver/pull/13675/commits/9e0872896e6f9b48786f9e04213cc8a746e9d6c4)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@trafficserver.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org