Issue166 by stevehu · Pull Request #167 · networknt/light-hybrid-4j

stevehu · 2026-01-27T22:48:54Z

No description provided.

for more information, see https://pre-commit.ci

Copilot

Pull request overview

This PR appears to address “Issue166” by updating configuration/module-registration integration and introducing request access-control support for RPC routing/security.

Changes:

Added a new AccessControlHandler middleware that evaluates rule-based access control per endpoint.
Updated router/security components to use newer *.load() config patterns and moved ModuleRegistry imports to com.networknt.server.
Updated Maven dependencies to include access-control/rule-loading related modules and server-config.

Reviewed changes

Copilot reviewed 11 out of 12 changed files in this pull request and generated 11 comments.

Show a summary per file

File	Description
rpc-security/src/main/java/com/networknt/rpc/security/HybridJwtVerifyHandler.java	Updates `ModuleRegistry` import and removes register/reload hooks (now leaving unused imports).
rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java	Adds new rule-based access control middleware for endpoint authorization.
rpc-security/pom.xml	Adds dependencies for access control + rule loader + server-config (introduces a duplicate `handler` dependency).
rpc-router/src/test/java/com/networknt/rpc/router/RpcRouterConfigTest.java	Updates tests to use `RpcRouterConfig.load(...)`.
rpc-router/src/main/resources/config/rpc-router-schema.json	Minor formatting change.
rpc-router/src/main/java/com/networknt/rpc/router/SchemaHandler.java	Refactors schema loading into `loadServices()` and leaves a commented-out `reload()` stub + unused import.
rpc-router/src/main/java/com/networknt/rpc/router/RpcStartupHookProvider.java	Removes module registration call, switches `ModuleRegistry` import (now unused).
rpc-router/src/main/java/com/networknt/rpc/router/RpcRouterConfig.java	Introduces cached singleton-style `load()` and module registration (thread-safety issue).
rpc-router/src/main/java/com/networknt/rpc/router/JsonHandler.java	Converts to `MiddlewareHandler` and switches to `ServerConfig.load()` (unused import added).
rpc-router/src/main/java/com/networknt/rpc/HybridHandler.java	Minor logging / style improvements.
rpc-router/pom.xml	Adds `server-config` dependency.
pom.xml	Adds dependency management entries for access/rule-related artifacts and `yaml-rule` version property.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

rpc-router/src/main/java/com/networknt/rpc/router/JsonHandler.java

rpc-router/src/main/java/com/networknt/rpc/router/SchemaHandler.java

rpc-security/src/main/java/com/networknt/rpc/security/HybridJwtVerifyHandler.java

Copilot · 2026-01-27T22:57:17Z

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

+
+import com.networknt.access.AccessControlConfig;
+import com.networknt.config.Config;
+import com.networknt.config.JsonMapper;


com.networknt.config.Config is imported but not used in this class. Please remove the unused import.

Suggested change

import com.networknt.config.JsonMapper;

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

rpc-router/src/main/java/com/networknt/rpc/router/RpcRouterConfig.java

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

rpc-security/pom.xml

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

rpc-router/src/main/java/com/networknt/rpc/router/SchemaHandler.java

…issue166

Copilot

Pull request overview

Copilot reviewed 11 out of 12 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (2)

rpc-router/src/main/java/com/networknt/rpc/router/JsonHandler.java:44

auditInfo can be null (you have an explicit branch for that), but later in this method auditInfo is still dereferenced (e.g., when extracting HYBRID_SERVICE_DATA), which will throw a NullPointerException. Either require/ensure AUDIT_INFO is always present for this handler, or guard all subsequent auditInfo.get(...) usages when falling back to ServerConfig.

        final ServerConfig serverConfig = ServerConfig.load();

        // get the serviceId from the auditInfo attachment.
        if (auditInfo != null) {
            serviceId = (String)auditInfo.get(Constants.ENDPOINT_STRING);

rpc-router/src/main/java/com/networknt/rpc/router/RpcStartupHookProvider.java:47

Please remove the System.out.println debug statement; it will write to stdout in production. Use the existing logger (or remove entirely) instead.

        final var packages = config.getHandlerPackages().toArray(new String[0]);
        System.out.println("packages: " + Arrays.toString(packages));

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-28T01:21:39Z

rpc-router/src/main/java/com/networknt/rpc/router/RpcStartupHookProvider.java

 import com.networknt.server.StartupHookProvider;
 import com.networknt.service.SingletonServiceFactory;
-import com.networknt.utility.ModuleRegistry;
+import com.networknt.server.ModuleRegistry;
 import io.github.classgraph.ClassGraph;


These imports appear unused after the refactor (e.g., Config and ModuleRegistry). Consider removing unused imports to keep the file clean and avoid IDE/CI warnings.

Copilot · 2026-01-28T01:21:40Z

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

+    static final String STARTUP_HOOK_NOT_LOADED = "ERR11019";
+    static final String REQUEST_ACCESS = "req-acc";
+    static final String PERMISSION = "permission";
+    static final String RULE_ID = "ruleId";


RULE_ID is declared but not used in this class. Please remove it if it’s not needed, or use it if it’s intended to be part of the requestRules structure.

Suggested change

static final String RULE_ID = "ruleId";

Copilot · 2026-01-28T01:21:40Z

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

+        boolean finalResult = false;  // Initialize to false for "any" logic, and will change in for loop
+        List<String> accessRuleIds = (List<String>)requestRules.get(REQUEST_ACCESS);
+        Map<String, Object> permissionMap = (Map<String, Object>)requestRules.get(PERMISSION);
+        Map<String, Object> result = null;


executeRules assumes requestRules always contains a non-null/non-empty req-acc list and a non-null permission map. If either is missing/empty, this can NPE and (for all logic with an empty list) can effectively allow access. Please validate these inputs and fail closed (return an access-control error) when they are missing/invalid.

Suggested change

boolean finalResult = false; // Initialize to false for "any" logic, and will change in for loop

List<String> accessRuleIds = (List<String>)requestRules.get(REQUEST_ACCESS);

Map<String, Object> permissionMap = (Map<String, Object>)requestRules.get(PERMISSION);

Map<String, Object> result = null;

// Validate requestRules to avoid NPE and fail closed on invalid configuration

if (requestRules == null) {

logger.error("Access control configuration is missing for this request: requestRules is null");

setExchangeStatus(exchange, ACCESS_CONTROL_ERROR, "Missing access control configuration");

return;

}

boolean finalResult = false; // Initialize to false for "any" logic, and will change in for loop

List<String> accessRuleIds = (List<String>) requestRules.get(REQUEST_ACCESS);

Map<String, Object> permissionMap = (Map<String, Object>) requestRules.get(PERMISSION);

Map<String, Object> result = null;

// Validate that we have a non-empty list of access rules and a permission map

if (accessRuleIds == null || accessRuleIds.isEmpty() || permissionMap == null) {

logger.error("Access control configuration is invalid: req-acc list or permission map is missing or empty");

setExchangeStatus(exchange, ACCESS_CONTROL_ERROR, "Invalid access control configuration");

return;

}

Copilot · 2026-01-28T01:21:40Z

rpc-security/src/main/java/com/networknt/rpc/security/AccessControlHandler.java

+    @Override
+    @SuppressWarnings("unchecked")
+    public void handleRequest(final HttpServerExchange exchange) throws Exception {
+        AccessControlConfig config = AccessControlConfig.load();
+        if (logger.isDebugEnabled()) logger.debug("AccessControlHandler.handleRequest starts.");
+        String reqPath = exchange.getRequestPath();
+        // if request path is in the skipPathPrefixes in the config, call the next handler directly to skip the security check.
+        if (config.getSkipPathPrefixes() != null && config.getSkipPathPrefixes().stream().anyMatch(reqPath::startsWith)) {
+            if(logger.isTraceEnabled()) logger.trace("Skip request path base on skipPathPrefixes for {}", reqPath);
+            if (logger.isDebugEnabled()) logger.debug("AccessControlHandler.handleRequest ends with path skipped.");
+            Handler.next(exchange, next);
+            return;
+        }
+


This PR introduces a new access-control decision point, but there are no tests covering its behavior (skipPathPrefixes, defaultDeny, any/all logic, and error status codes). Please add unit/integration tests to prevent regressions.

stevehu added 3 commits January 25, 2026 18:37

fixes #166 refactor config to central location to support reload

50580b3

remove register and check configMap for reload

b5a4063

fixes #166 refactor handler and config

e7e5bac

stevehu requested a review from Copilot January 27, 2026 22:48

[pre-commit.ci] auto fixes from pre-commit.com hooks

de70cce

for more information, see https://pre-commit.ci

Copilot started reviewing on behalf of stevehu January 27, 2026 22:49 View session

Copilot AI reviewed Jan 27, 2026

View reviewed changes

stevehu added 2 commits January 27, 2026 20:11

fixes #166 update to fix the handlers and configs

1e1f361

Merge branch 'issue166' of github.com:networknt/light-hybrid-4j into …

fcc5d66

…issue166

stevehu requested a review from Copilot January 28, 2026 01:13

Copilot started reviewing on behalf of stevehu January 28, 2026 01:13 View session

stevehu merged commit 27312b1 into master Jan 28, 2026
5 checks passed

stevehu deleted the issue166 branch January 28, 2026 01:15

Copilot AI reviewed Jan 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue166#167

Issue166#167
stevehu merged 6 commits intomasterfrom
issue166

stevehu commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 27, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Copilot AI Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-        boolean finalResult = false;  // Initialize to false for "any" logic, and will change in for loop
-        List<String> accessRuleIds = (List<String>)requestRules.get(REQUEST_ACCESS);
-        Map<String, Object> permissionMap = (Map<String, Object>)requestRules.get(PERMISSION);
-        Map<String, Object> result = null;
+        // Validate requestRules to avoid NPE and fail closed on invalid configuration
+        if (requestRules == null) {
+            logger.error("Access control configuration is missing for this request: requestRules is null");
+            setExchangeStatus(exchange, ACCESS_CONTROL_ERROR, "Missing access control configuration");
+            return;
+        }
+        boolean finalResult = false;  // Initialize to false for "any" logic, and will change in for loop
+        List<String> accessRuleIds = (List<String>) requestRules.get(REQUEST_ACCESS);
+        Map<String, Object> permissionMap = (Map<String, Object>) requestRules.get(PERMISSION);
+        Map<String, Object> result = null;
+        // Validate that we have a non-empty list of access rules and a permission map
+        if (accessRuleIds == null || accessRuleIds.isEmpty() || permissionMap == null) {
+            logger.error("Access control configuration is invalid: req-acc list or permission map is missing or empty");
+            setExchangeStatus(exchange, ACCESS_CONTROL_ERROR, "Invalid access control configuration");
+            return;
+        }

Conversation

stevehu commented Jan 27, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 27, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants