Open
Conversation
WalkthroughThis pull request introduces a mediator access control system for Apache Synapse that enforces configuration-driven allowlist/blocklist policies during XML-based mediator deployment. It adds infrastructure for policy enforcement, error handling, and integrates access checks into the mediator factory initialization and instantiation flows. Changes
Sequence Diagram(s)sequenceDiagram
participant SynapseConfig as Synapse Config
participant MFF as MediatorFactoryFinder
participant MAC as MediatorAccessControl
participant Factory as MediatorFactory
participant ALM as AbstractListMediatorFactory
SynapseConfig->>MFF: loadMediatorFactories()
MFF->>MFF: Register built-in & pluggable factories
MFF->>MAC: init()
MAC->>MAC: Load config from synapse.properties
MFF->>MFF: Set initialized = true
SynapseConfig->>MFF: getMediator(OMElement, ...)
MFF->>MAC: checkByElementName(localName)
alt Blocked by Policy
MAC-->>MFF: throw MediatorAccessControlException
else Allowed
MAC-->>MFF: OK
end
MFF->>Factory: Instantiate mediator
SynapseConfig->>ALM: addChildren(parent, mediators)
loop For each child
ALM->>MFF: getMediator(childElement, ...)
alt Access Denied
MFF-->>ALM: MediatorAccessControlException
ALM->>ALM: Accumulate blocked name
else Success
MFF-->>ALM: Mediator instance
ALM->>ALM: Add to parent
end
end
ALM->>ALM: If any blocked, log consolidated message
alt Blocked mediators exist
ALM-->>SynapseConfig: throw SynapseException
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
modules/core/src/main/java/org/apache/synapse/config/xml/AbstractListMediatorFactory.java (1)
50-57: Move configuration extraction outside the loop for efficiency.The
SynapseConfigurationextraction fromproperties(lines 50-55) is performed on every loop iteration, but the result is the same each time. This should be moved outside thewhileloop.♻️ Proposed refactor
protected static void addChildren(OMElement el, ListMediator m, Properties properties) { Iterator it = el.getChildren(); StringBuilder blockedMediators = null; + SynapseConfiguration configuration = null; + if (properties != null) { + configuration = properties.get(SynapseConstants.SYNAPSE_CONFIGURATION) != null + ? (SynapseConfiguration) properties.get(SynapseConstants.SYNAPSE_CONFIGURATION) + : null; + } while (it.hasNext()) { OMNode child = (OMNode) it.next(); if (child instanceof OMElement) { if (!DESCRIPTION_Q.equals(((OMElement) child).getQName())) { // neglect the description tag try { - SynapseConfiguration configuration = null; - if (properties != null) { - configuration = properties.get(SynapseConstants.SYNAPSE_CONFIGURATION) != null - ? (SynapseConfiguration) properties.get(SynapseConstants.SYNAPSE_CONFIGURATION) - : null; - } Mediator med = MediatorFactoryFinder.getInstance().getMediator((OMElement) child, properties, configuration);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/core/src/main/java/org/apache/synapse/config/xml/AbstractListMediatorFactory.java` around lines 50 - 57, The code repeatedly extracts SynapseConfiguration from the properties map inside the loop; move the extraction of SynapseConfiguration (using properties.get(SynapseConstants.SYNAPSE_CONFIGURATION) cast to SynapseConfiguration) to before the loop in AbstractListMediatorFactory so the same configuration variable is reused for each iteration, then call MediatorFactoryFinder.getInstance().getMediator((OMElement) child, properties, configuration) inside the loop without re-reading properties on every pass.modules/core/src/main/java/org/apache/synapse/config/xml/MediatorAccessControl.java (2)
147-159: Consider elevating log level for security audit events.Blocked mediator attempts are security-relevant events that may warrant audit logging. The current
DEBUGlevel (lines 150, 156) might not be enabled in production environments, potentially missing important security audit trails.♻️ Proposed change to use INFO level for audit
if (AccessControlListType.BLOCK_LIST == mediatorAccessControlConfig.listType) { if (mediatorAccessControlConfig.mediators.contains(normalizedName)) { String msg = "Mediator '" + displayName + "' is blocked by mediator access control."; - log.debug(msg); + if (log.isInfoEnabled()) { + log.info(msg); + } throw new MediatorAccessControlException(msg, displayName); } } else if (AccessControlListType.ALLOW_LIST == mediatorAccessControlConfig.listType) { if (!mediatorAccessControlConfig.mediators.contains(normalizedName)) { String msg = "Mediator '" + displayName + "' is not in the allowed mediators list."; - log.debug(msg); + if (log.isInfoEnabled()) { + log.info(msg); + } throw new MediatorAccessControlException(msg, displayName); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/core/src/main/java/org/apache/synapse/config/xml/MediatorAccessControl.java` around lines 147 - 159, The security-relevant blocked/denied mediator events in MediatorAccessControl (the checks using mediatorAccessControlConfig.listType and the normalizedName/displayName variables) are currently logged with log.debug; change these to a higher audit level (use log.info) for both the BLOCK_LIST and ALLOW_LIST branches where you construct the msg and before throwing MediatorAccessControlException so denied attempts are recorded in production logs.
70-70: Consider markingconfigasvolatilefor defensive clarity.While the current usage appears safe due to the synchronized initialization in
MediatorFactoryFinder.getInstance()establishing a happens-before relationship, marking the field asvolatilewould make the thread-safety intent explicit and guard against future refactoring that might bypass the synchronized path.♻️ Proposed change
- private static MediatorAccessControlConfig config = MediatorAccessControlConfig.NONE; + private static volatile MediatorAccessControlConfig config = MediatorAccessControlConfig.NONE;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@modules/core/src/main/java/org/apache/synapse/config/xml/MediatorAccessControl.java` at line 70, Mark the static field 'config' in MediatorAccessControl as volatile to make the concurrency intent explicit: change the declaration of MediatorAccessControl.config (type MediatorAccessControlConfig) to be volatile so reads/writes have the proper memory visibility guarantees established by MediatorFactoryFinder.getInstance(); this prevents future refactorings from accidentally breaking the happens-before assumptions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@modules/core/src/main/java/org/apache/synapse/config/xml/AbstractListMediatorFactory.java`:
- Around line 50-57: The code repeatedly extracts SynapseConfiguration from the
properties map inside the loop; move the extraction of SynapseConfiguration
(using properties.get(SynapseConstants.SYNAPSE_CONFIGURATION) cast to
SynapseConfiguration) to before the loop in AbstractListMediatorFactory so the
same configuration variable is reused for each iteration, then call
MediatorFactoryFinder.getInstance().getMediator((OMElement) child, properties,
configuration) inside the loop without re-reading properties on every pass.
In
`@modules/core/src/main/java/org/apache/synapse/config/xml/MediatorAccessControl.java`:
- Around line 147-159: The security-relevant blocked/denied mediator events in
MediatorAccessControl (the checks using mediatorAccessControlConfig.listType and
the normalizedName/displayName variables) are currently logged with log.debug;
change these to a higher audit level (use log.info) for both the BLOCK_LIST and
ALLOW_LIST branches where you construct the msg and before throwing
MediatorAccessControlException so denied attempts are recorded in production
logs.
- Line 70: Mark the static field 'config' in MediatorAccessControl as volatile
to make the concurrency intent explicit: change the declaration of
MediatorAccessControl.config (type MediatorAccessControlConfig) to be volatile
so reads/writes have the proper memory visibility guarantees established by
MediatorFactoryFinder.getInstance(); this prevents future refactorings from
accidentally breaking the happens-before assumptions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 122d86cb-0467-4708-8f1b-c719f3b64b5e
📒 Files selected for processing (4)
modules/core/src/main/java/org/apache/synapse/config/xml/AbstractListMediatorFactory.javamodules/core/src/main/java/org/apache/synapse/config/xml/MediatorAccessControl.javamodules/core/src/main/java/org/apache/synapse/config/xml/MediatorAccessControlException.javamodules/core/src/main/java/org/apache/synapse/config/xml/MediatorFactoryFinder.java
PasanT9
approved these changes
Apr 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Purpose
This PR adds Mediator Access Control.
Summary by CodeRabbit