Copilot commented on code in PR #1302:
URL: https://github.com/apache/ranger/pull/1302#discussion_r4222939581
##########
agents-common/src/main/java/org/apache/ranger/plugin/util/RangerAccessRequestUtil.java:
##########
@@ -60,11 +61,18 @@ public class RangerAccessRequestUtil {
public static final String KEY_CONTEXT_BATCH_EVAL_CONTEXT =
"BATCH_EVAL_CONTEXT";
public static final String KEY_CONTEXT_RESOURCE_ZONE_NAMES =
"RESOURCE_ZONE_NAMES";
public static final String KEY_CONTEXT_ACL_ENFORCER =
"_ACL_ENFORCER";
+ // make sure to add all internal keys in INTERNAL_CONTEXT_KEYS, to exclude
them from CTX_ATTR in expressions
+
+ private static final Set<String> INTERNAL_CONTEXT_KEYS;
private RangerAccessRequestUtil() {
// to avoid instantiation
}
+ public static boolean isInternalKey(String key) {
+ return INTERNAL_CONTEXT_KEYS.contains(key);
+ }
Review Comment:
`KEY_TOKEN_NAMESPACE` is a prefix (`"token:"`), but `contains()` only
filters a key exactly equal to that prefix. Keys created by
`setTokenInContext()` (for example `token:USER`, `token:ROLES`, and HDFS
filename tokens) therefore leak into `CTX_ATTR`, contrary to this internal-key
filter. Treat every key beginning with the namespace as internal, and add a
regression assertion for a namespaced token.
##########
agents-common/src/test/java/org/apache/ranger/plugin/conditionevaluator/RangerRequestScriptEvaluatorTest.java:
##########
@@ -605,6 +605,40 @@ void testBlockJavaClassReferencesWithDefaultConfig() {
Assertions.assertTrue(escaped.isEmpty(), "Sandbox escape OS command
executed via:\n " + String.join("\n ", escaped));
}
+ @Test
+ public void testContextAttributes() {
+ Map<String, Object> attributes = new HashMap<>();
+
+ attributes.put("trx_amount", 10000);
+
+ RangerAccessRequest request = createRequest("test-user",
Collections.emptySet(), Collections.emptySet(), attributes);
+ RangerRequestScriptEvaluator evaluator = new
RangerRequestScriptEvaluator(request, scriptEngine);
+
+ Assertions.assertTrue((Boolean)
evaluator.evaluateScript("CTX_ATTR.trx_amount <= 10000"), "CTX_ATTR.trx_amount
<= 10000");
Review Comment:
This test constructs the evaluator with the overload that always enables the
JSON context, so it would still pass if `CTX_ATTR` were omitted from
`needsJsonCtxEnabled()`—the path used by `RangerScriptConditionEvaluator` in
production. Add an assertion for automatic detection (or exercise
`RangerScriptConditionEvaluator`) so the new variable cannot silently evaluate
without initialization.
--
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]
