Copilot commented on code in PR #12950:
URL: https://github.com/apache/trafficserver/pull/12950#discussion_r2907353300
##########
tools/hrw4u/src/visitor.py:
##########
@@ -318,6 +336,7 @@ def visitVarSection(self, ctx) -> None:
else:
raise error
with self.debug_context("visitVarSection"):
+ self._sandbox.check_language("variables")
self.visit(ctx.variables())
Review Comment:
`visitVarSection()` calls `self._sandbox.check_language("variables")`
outside of `self.trap(ctx)`. If variables are denied and an `error_collector`
is present, this denial will raise and abort compilation instead of being
collected as a normal compilation error. Wrapping the sandbox check (or the
whole method body) in `self.trap(ctx)` would make sandbox denials consistent
with other checks.
##########
tools/hrw4u/src/symbols.py:
##########
@@ -138,6 +143,8 @@ def resolve_condition(self, name: str, section: SectionType
| None = None) -> tu
if symbol := self.symbol_for(name):
return symbol.as_cond(), False
+ self._sandbox.check_condition(name)
+
Review Comment:
`resolve_condition()` returns declared variables via `symbol_for()` before
any sandbox checks. This means `sandbox.deny.language: [variables]` (and any
future variable-related policy) won’t actually block variable *usage* in
conditions/interpolation, only the `VARS` section. Consider enforcing the
sandbox policy when a declared variable is referenced (e.g., check
`sandbox.check_language("variables")` before returning `symbol.as_cond()`).
##########
tools/hrw4u/src/visitor.py:
##########
@@ -49,14 +50,29 @@ def __init__(
filename: str = SystemDefaults.DEFAULT_FILENAME,
debug: bool = SystemDefaults.DEFAULT_DEBUG,
error_collector=None,
- preserve_comments: bool = True) -> None:
+ preserve_comments: bool = True,
+ sandbox: SandboxConfig | None = None) -> None:
super().__init__(filename, debug, error_collector)
self._cond_state = CondState()
self._queued: QueuedItem | None = None
self.preserve_comments = preserve_comments
+ self._sandbox = sandbox or SandboxConfig.empty()
- self.symbol_resolver = SymbolResolver(debug)
+ self.symbol_resolver = SymbolResolver(debug, sandbox=self._sandbox)
+
+ def _sandbox_check(self, ctx, check_fn) -> bool:
+ """Run a sandbox check, trapping any denial error into the error
collector.
+
+ Returns True if the check passed, False if denied (error was
collected).
+ """
+ try:
+ check_fn()
+ return True
+ except Exception as exc:
+ with self.trap(ctx):
+ raise exc
+ return False
Review Comment:
`_sandbox_check()` catches `Exception` and then re-raises via `raise exc`.
Since this helper is meant for sandbox denials, catching broadly can hide
unrelated programming errors and `raise exc` also discards the original
traceback. It would be safer to catch `SandboxDenialError` explicitly and use
bare `raise` to preserve traceback/context.
##########
tools/hrw4u/schema/sandbox.schema.json:
##########
@@ -0,0 +1,125 @@
+{
+ "$schema": "http://json-schema.org/draft-07/schema#";,
+ "$id": "https://trafficserver.apache.org/schemas/hrw4u-sandbox.schema.json";,
+ "title": "HRW4U Sandbox Configuration",
+ "description": "Policy deny-list for the hrw4u compiler (--sandbox FILE).",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "sandbox": {
+ "type": "object",
Review Comment:
The schema allows `{}` because it doesn’t mark `sandbox` as required, but
`SandboxConfig.load()` and the docs treat a top-level `sandbox` mapping as
mandatory. Add `"required": ["sandbox"]` to the schema, or relax the loader so
configs without `sandbox` are treated as empty/allowed.
##########
doc/admin-guide/configuration/hrw4u.en.rst:
##########
@@ -494,6 +542,137 @@ Run with `--debug all` to trace:
- Condition evaluations
- State and output emission
+Sandbox Policy Enforcement
+==========================
+
+Organizations deploying HRW4U across teams can restrict which language features
+are permitted using a sandbox configuration file. When a denied feature is
used,
+the compiler emits a clear error with a configurable message explaining the
+restriction.
+
+Pass the sandbox file with ``--sandbox``:
+
+.. code-block:: none
+
+ hrw4u --sandbox /etc/trafficserver/hrw4u-sandbox.yaml rules.hrw4u
+
+The sandbox file is YAML with a single top-level ``sandbox`` key. A JSON
+Schema for editor validation and autocomplete is provided at
+``tools/hrw4u/schema/sandbox.schema.json``.
+
+.. code-block:: yaml
+
+ sandbox:
+ message: | # optional: shown once after all denial errors
+ ...
+ deny:
+ sections: [ ... ] # section names, e.g. TXN_START
+ functions: [ ... ] # function names, e.g. run-plugin
+ conditions: [ ... ] # condition keys, e.g. geo.
+ operators: [ ... ] # operator keys, e.g. inbound.conn.dscp
+ language: [ ... ] # break, variables, in, else, elif
+
+All deny lists are optional. An empty or missing sandbox file permits
everything.
+
+Sections
+--------
+
+The ``sections`` list accepts any of the HRW4U section names listed in the
Review Comment:
The `sandbox.deny.language` documentation only lists `break` and
`variables`, but the implementation/schema also supports `in`, `else`, and
`elif` (and tests cover them). Please expand this section/table to include all
supported constructs so policy authors can configure denials correctly.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
