Skip to content

Commit e9f1cfd

Browse files
authored
fix: dicht systemische template-injectie in OPI-manifesten (#77)
* fix: dicht systemische template-injectie in OPI-manifesten
1 parent 672a11f commit e9f1cfd

6 files changed

Lines changed: 347 additions & 55 deletions

File tree

operations-manager/python/manifests/argocd-appproject.yaml.jinja

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ spec:
1616
destinations:
1717
- namespace: {{ destination.namespace }}
1818
server: "https://kubernetes.default.svc"
19-
clusterResourceWhitelist:
20-
- group: '*'
21-
kind: '*'
19+
# Cluster-scoped dicht; namespaced open want destinations.namespace pint al de scope.
20+
clusterResourceWhitelist: []
2221
namespaceResourceWhitelist:
2322
- group: '*'
2423
kind: '*'

operations-manager/python/manifests/deployment.yaml.jinja

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ spec:
5353
type: RuntimeDefault
5454
containers:
5555
- name: app
56-
image: "{{ imageURL }}"
56+
image: {{ (imageURL | default('', true)) | tojson }}
5757
imagePullPolicy: {{ imagePullPolicy }}
5858
ports:
5959
- containerPort: {{ application_port }}
@@ -76,14 +76,14 @@ spec:
7676
env:
7777
{% if env_vars %}
7878
{% for env_name, env_value in env_vars.items() %}
79-
- name: {{ env_name }}
80-
value: "{{ env_value }}"
79+
- name: {{ env_name | tojson }}
80+
value: {{ env_value | tojson }}
8181
{% endfor %}
8282
{% endif %}
8383
{% if ca_config and ca_config.enabled and ca_config.env_vars %}
8484
{% for env_name, env_value in ca_config.env_vars.items() %}
85-
- name: {{ env_name }}
86-
value: "{{ env_value }}"
85+
- name: {{ env_name | tojson }}
86+
value: {{ env_value | tojson }}
8787
{% endfor %}
8888
{% endif %}
8989
{% endif %}
@@ -98,8 +98,8 @@ spec:
9898
volumeMounts:
9999
{% if storage_configs %}
100100
{% for storage in storage_configs %}
101-
- name: {{ storage.name }}
102-
mountPath: {{ storage['mount-path'] }}
101+
- name: {{ storage.name | tojson }}
102+
mountPath: {{ storage['mount-path'] | tojson }}
103103
{% endfor %}
104104
{% endif %}
105105
{% if ca_config and ca_config.enabled %}
@@ -149,13 +149,13 @@ spec:
149149
{% if storage_configs %}
150150
{% for storage in storage_configs %}
151151
{% if storage.type == 'persistent' %}
152-
- name: {{ storage.name }}
152+
- name: {{ storage.name | tojson }}
153153
persistentVolumeClaim:
154-
claimName: {{ storage.pvc_name }}
154+
claimName: {{ storage.pvc_name | tojson }}
155155
{% elif storage.type == 'ephemeral' %}
156-
- name: {{ storage.name }}
156+
- name: {{ storage.name | tojson }}
157157
emptyDir:
158-
sizeLimit: {{ storage.size }}
158+
sizeLimit: {{ storage.size | tojson }}
159159
{% endif %}
160160
{% endfor %}
161161
{% endif %}

operations-manager/python/manifests/ingress.yaml.jinja

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ metadata:
99
haproxy.router.openshift.io/ip_whitelist: {{ ip_whitelist | default("0.0.0.0/0,::/0") }}
1010
haproxy.router.openshift.io/timeout: "300s"
1111
{% if rewrite %}
12-
haproxy.router.openshift.io/rewrite-target: {{ rewrite }}
12+
haproxy.router.openshift.io/rewrite-target: {{ (rewrite | default('', true)) | tojson }}
1313
{% endif %}
1414
{% if enable_tls %}
1515
# HSTS for internet.nl compliance (1 year, includeSubDomains, preload)
@@ -47,6 +47,7 @@ metadata:
4747
nginx.ingress.kubernetes.io/configuration-snippet: |
4848
{% if rewrite %}
4949
{% set rewrite_base = '' if rewrite == '/' else rewrite %}
50+
# path/rewrite_base zijn aan de bron charset-gevalideerd (_SAFE_PATH_PATTERN).
5051
rewrite "^{{ path }}/?(.*)$" "{{ rewrite_base }}/$1" break;
5152
{% endif %}
5253
more_set_headers "X-Content-Type-Options: nosniff";
@@ -65,26 +66,27 @@ metadata:
6566
# ============================================================
6667
# External-DNS target - overrides the default LB-status hostname
6768
# ============================================================
68-
external-dns.alpha.kubernetes.io/target: {{ external_dns_target }}
69+
external-dns.alpha.kubernetes.io/target: {{ (external_dns_target | default('', true)) | tojson }}
6970
{% endif %}
71+
{% set effective_service_name = service_name if service_name is defined and service_name else (name if name is defined else '') %}
7072
spec:
7173
rules:
72-
- host: {{ hostname }}
74+
- host: {{ (hostname | default('', true)) | tojson }}
7375
http:
7476
paths:
75-
- path: {{ path | default('/') }}
77+
- path: {{ (path | default('/', true)) | tojson }}
7678
pathType: Prefix
7779
backend:
7880
service:
79-
name: {{ service_name | default(name) }}
81+
name: {{ effective_service_name | tojson }}
8082
port:
8183
number: {{ service_port | default(80) }}
8284
{% if enable_tls %}
8385
tls:
8486
{% if issuer_name or cluster_issuer %}
8587
- hosts:
86-
- {{ hostname }}
87-
secretName: {{ tls_secret_name }}
88+
- {{ (hostname | default('', true)) | tojson }}
89+
secretName: {{ (tls_secret_name | default('', true)) | tojson }}
8890
{% else %}
8991
- {}
9092
{% endif %}

operations-manager/python/manifests/service.yaml.jinja

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
apiVersion: v1
22
kind: Service
33
metadata:
4-
name: "{{ name }}"
5-
namespace: "{{ namespace }}"
4+
name: {{ (name | default('', true)) | tojson }}
5+
namespace: {{ (namespace | default('', true)) | tojson }}
66
labels:
7-
app: "{{ name }}"
8-
project: "{{ project.name }}"
7+
app: {{ (name | default('', true)) | tojson }}
8+
project: {{ (project.name | default('', true)) | tojson }}
99
spec:
1010
selector:
11-
app: "{{ name }}"
11+
app: {{ (name | default('', true)) | tojson }}
1212
component: application
1313
ports:
1414
- port: {{ service_port | default(80) }}

operations-manager/python/opi/handlers/project_file_handler.py

Lines changed: 67 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -26,32 +26,6 @@
2626
logger = logging.getLogger(__name__)
2727

2828

29-
def _normalize_path_config(path_config: Any, label: str = "unknown") -> list[dict[str, str | None]]:
30-
"""Normalize a path value (string or list-of-dicts) to canonical list format.
31-
32-
Handles:
33-
- ``str``: pre-migration plain string path (backward compat)
34-
- ``list[dict]``: canonical v2.2+ format with ``match`` and optional ``rewrite``
35-
"""
36-
if isinstance(path_config, str):
37-
logger.debug("Found single path '%s' for '%s'", path_config, label)
38-
return [{"match": path_config, "rewrite": None}]
39-
40-
if isinstance(path_config, list):
41-
result = []
42-
for p in path_config:
43-
if isinstance(p, dict):
44-
result.append({"match": p.get("match", "/"), "rewrite": p.get("rewrite")})
45-
else:
46-
result.append({"match": str(p), "rewrite": None})
47-
if result:
48-
logger.debug("Found %d path(s) for '%s'", len(result), label)
49-
return result
50-
51-
logger.debug("No path found for '%s', using default '/'", label)
52-
return [{"match": "/", "rewrite": None}]
53-
54-
5529
# Default resource values for deployment containers
5630
DEFAULT_RESOURCES: dict[str, str] = {
5731
"requests_memory": "128Mi",
@@ -89,6 +63,53 @@ def _parse_resources_block(raw: dict | None, defaults: dict[str, str] | None = N
8963
}
9064

9165

66+
# URL-path charset voor tenant-bestuurde component match/rewrite waarden;
67+
# voorkomt nginx-snippet-injection in ingress.yaml.jinja.
68+
_SAFE_PATH_PATTERN = re.compile(r"^/[A-Za-z0-9/_.~-]*$")
69+
70+
71+
def _sanitize_path_value(value: Any, field: str) -> str:
72+
"""
73+
Validate a tenant-supplied URL path used for ingress routing/rewriting.
74+
75+
Raises:
76+
ValueError: If the value is not a string or contains characters
77+
outside the safe URL-path charset (defense against nginx
78+
configuration-snippet injection).
79+
"""
80+
if not isinstance(value, str):
81+
raise ValueError( # noqa: TRY004
82+
f"Component path '{field}' must be a string, got {type(value).__name__}"
83+
)
84+
if not value:
85+
raise ValueError(f"Component path '{field}' must not be empty")
86+
if not _SAFE_PATH_PATTERN.match(value):
87+
raise ValueError(
88+
f"Component path '{field}' contains illegal characters: {value!r}. "
89+
f"Only '/', letters, digits and '-._~' are allowed (no whitespace, "
90+
f"quotes or newlines)."
91+
)
92+
return value
93+
94+
95+
def _normalize_path_config(raw: Any, default_match: str = "/") -> dict[str, str | None]:
96+
"""
97+
Build a validated {"match": ..., "rewrite": ...} entry from a raw path item.
98+
99+
Both `match` and `rewrite` are sanitized against the safe URL-path charset.
100+
"""
101+
if isinstance(raw, dict):
102+
match_value = raw.get("match", default_match)
103+
rewrite_value = raw.get("rewrite")
104+
else:
105+
match_value = raw
106+
rewrite_value = None
107+
108+
sanitized_match = _sanitize_path_value(match_value, "match")
109+
sanitized_rewrite = None if rewrite_value is None else _sanitize_path_value(rewrite_value, "rewrite")
110+
return {"match": sanitized_match, "rewrite": sanitized_rewrite}
111+
112+
92113
def _parse_resources_block_partial(raw: dict | None) -> dict[str, str]:
93114
"""
94115
Parse a nested resources block into flat keys without filling defaults.
@@ -665,7 +686,18 @@ def extract_component_paths(self, project_data: dict[str, Any], component_name:
665686
"""
666687
json_path = f"$.components[?(@.name='{component_name}')].path"
667688
path_config = self.extract_value_by_path(project_data, json_path, "/")
668-
return _normalize_path_config(path_config, component_name)
689+
690+
# Normaliseer naar list-format; sanitizer valideert per item.
691+
if isinstance(path_config, str):
692+
logger.debug(f"Found single path '{path_config}' for component '{component_name}'")
693+
return [_normalize_path_config(path_config)]
694+
if isinstance(path_config, list):
695+
result = [_normalize_path_config(p) for p in path_config]
696+
logger.info(f"Found {len(result)} path(s) for component '{component_name}'")
697+
return result
698+
699+
logger.debug(f"No path found for component '{component_name}', using default '/'")
700+
return [{"match": "/", "rewrite": None}]
669701

670702
def extract_deployment_component_paths(
671703
self,
@@ -691,10 +723,16 @@ def extract_deployment_component_paths(
691723
components = deployment_data.get("components", [])
692724
for comp in components:
693725
if comp.get("reference") == component_reference:
726+
# Schema: singular `path` (str | list); normaliseer via sanitizer.
694727
path_config = comp.get("path")
695728
if path_config is not None:
696-
result = _normalize_path_config(path_config, component_reference)
697-
if result != [{"match": "/", "rewrite": None}]:
729+
if isinstance(path_config, str):
730+
result = [_normalize_path_config(path_config)]
731+
elif isinstance(path_config, list):
732+
result = [_normalize_path_config(p) for p in path_config]
733+
else:
734+
result = [{"match": "/", "rewrite": None}]
735+
if result and result != [{"match": "/", "rewrite": None}]:
698736
logger.info(
699737
f"Found {len(result)} deployment-level path(s) for component '{component_reference}'"
700738
)

0 commit comments

Comments
 (0)