[Fixes #14245] Management command to clear GWC cache#14246
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new Django management command gwc with a truncate subcommand to clear GeoWebCache (GWC) for specific layers or all layers. The implementation includes the command structure, the core truncation logic, and unit tests. A significant security vulnerability was identified in the truncate_layers function, where unescaped layer names could lead to XML injection. It is recommended to refactor this function to escape user input and improve error handling for failed API requests.
| from geonode.geoserver.helpers import _invalidate_geowebcache_dataset, ogc_server_settings, _user, http_client | ||
|
|
||
| logger = setup_logger() | ||
|
|
||
|
|
||
| def truncate_layers(layer_names): | ||
| """Truncates one or more specified layers in GWC.""" | ||
| logger.info(f"Truncating layers in GWC: {', '.join(layer_names)}") | ||
| for layer_name in layer_names: | ||
| try: | ||
| logger.info(f"Truncating layer: {layer_name}") | ||
| _invalidate_geowebcache_dataset(layer_name) | ||
| except Exception as e: | ||
| logger.error(f"Error invalidating cache for layer {layer_name}: {e}") |
There was a problem hiding this comment.
The truncate_layers function calls _invalidate_geowebcache_dataset, which has two issues:
-
XML Injection Vulnerability: It constructs XML via f-string without escaping user-provided layer names. This could allow a malicious layer name to alter the request and perform unintended actions, like truncating all caches.
-
Insufficient Error Handling: It logs failed requests (non-200 status) at the
debuglevel and doesn't raise an exception. This means failures will not be clearly reported.
To address this, I suggest rewriting truncate_layers to handle the API call directly. This allows for proper XML escaping and better error reporting, making the command more secure and reliable.
Note: This change will require updating the mock path in test_truncate_layer_calls_api in geonode/geoserver/tests/test_gwc.py.
from xml.sax.saxutils import escape
from geonode.geoserver.helpers import ogc_server_settings, _user, http_client
logger = setup_logger()
def truncate_layers(layer_names):
"""Truncates one or more specified layers in GWC."""
logger.info(f"Truncating layers in GWC: {', '.join(layer_names)}")
url = f"{ogc_server_settings.LOCATION}gwc/rest/masstruncate"
headers = {"Content-Type": "text/xml"}
for layer_name in layer_names:
try:
logger.info(f"Truncating layer: {layer_name}")
body = f"<truncateLayer><layerName>{escape(layer_name)}</layerName></truncateLayer>"
req, content = http_client.post(url, data=body, headers=headers, user=_user)
if req.status_code != 200:
logger.error(f"Error {req.status_code} invalidating cache for layer {layer_name}: {content}")
except Exception as e:
logger.error(f"Error invalidating cache for layer {layer_name}: {e}")
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #14246 +/- ##
===========================================
- Coverage 74.92% 44.05% -30.88%
===========================================
Files 975 978 +3
Lines 59902 59996 +94
Branches 8157 8170 +13
===========================================
- Hits 44884 26429 -18455
- Misses 13194 32270 +19076
+ Partials 1824 1297 -527 🚀 New features to boost your workflow:
|
2 participants
Fixes #14245
Checklist
For all pull requests:
The following are required only for core and extension modules (they are welcomed, but not required, for contrib modules):
Submitting the PR does not require you to check all items, but by the time it gets merged, they should be either satisfied or inapplicable.