From b9e069301fb499f394a9d5ef3ce92c38355165ba Mon Sep 17 00:00:00 2001 From: KaifAhmad1 Date: Wed, 24 Jun 2026 22:55:18 +0530 Subject: [PATCH] fix(deploy): address security and correctness blockers from PR review - gcp/cloudrun-service.yaml: add comment + README sed one-liner so PROJECT_ID is substituted before gcloud run services replace (was a literal placeholder that caused image-pull failure on the declarative deploy path) - azure/main.parameters.json: replace wildcard allowedOrigins "*" with a REPLACE_ME placeholder; add README note to set the real URL after first deploy - kubernetes/networkpolicy.yaml + helm networkpolicy template: add from: selector (ingress-nginx namespace + same-namespace pods) so ingress is no longer allow-all; restrict egress to FalkorDB port 6379 and DNS port 53 instead of the allow-all egress: - {} wildcard - helm/values.yaml: expose networkPolicy.ingressNamespace and falkordbPort values - kubernetes/deployment.yaml: add secretRef for knowledge-explorer-secrets so FALKORDB_PASSWORD is actually injected into the container - app.py: add _mutation_bridge_installed guard to prevent closure stacking when the same GraphSession is passed to create_app() more than once; remove duplicate app.state.allowed_origins assignment (single source of truth is app.state.explorer_settings); add comment on falkordb_host/port dead config - tests: update allowed_origins assertions to use explorer_settings dict - .checkov.yaml: remove global CKV_K8S_21/28/30 suppressions; rely on per-file inline checkov:skip comments in cloudrun-service.yaml so future real K8s manifests are not silently exempted --- .checkov.yaml | 21 +++------------- deploy/azure/README.md | 2 ++ deploy/azure/main.parameters.json | 2 +- deploy/gcp/README.md | 5 ++-- deploy/gcp/cloudrun-service.yaml | 2 ++ .../templates/networkpolicy.yaml | 25 +++++++++++++++++-- deploy/helm/knowledge-explorer/values.yaml | 5 ++++ deploy/kubernetes/deployment.yaml | 3 +++ deploy/kubernetes/networkpolicy.yaml | 25 +++++++++++++++++-- semantica/explorer/app.py | 7 +++++- tests/explorer/test_explorer_api.py | 4 +-- 11 files changed, 74 insertions(+), 27 deletions(-) diff --git a/.checkov.yaml b/.checkov.yaml index b5f8058e..d0136e7c 100644 --- a/.checkov.yaml +++ b/.checkov.yaml @@ -1,17 +1,4 @@ -# Checkov configuration — applies when running checkov locally or via the -# bridgecrewio/checkov-action in CI. -# -# The rules below are suppressed because deploy/gcp/cloudrun-service.yaml uses -# apiVersion: serving.knative.dev/v1 which checkov scans as a Kubernetes resource. -# Cloud Run is a fully managed platform — K8s namespace, AppArmor, and seccomp -# concepts do not apply to it. -# -# CKV_K8S_21 — "default namespace should not be used": Cloud Run services have no -# namespace concept; checkov sees the absence of metadata.namespace -# as using the "default" namespace. -# CKV_K8S_28 — seccomp profile: Cloud Run enforces this at the platform level. -# CKV_K8S_30 — AppArmor profile: Cloud Run enforces this at the platform level. -skip-check: - - CKV_K8S_21 - - CKV_K8S_28 - - CKV_K8S_30 +# Checkov configuration. +# Cloud Run false-positives (CKV_K8S_21/28/30) are suppressed via per-file +# inline checkov:skip comments in deploy/gcp/cloudrun-service.yaml rather than +# globally here, so real Kubernetes manifests are not silently exempted. diff --git a/deploy/azure/README.md b/deploy/azure/README.md index d0fbf4c7..078871c4 100644 --- a/deploy/azure/README.md +++ b/deploy/azure/README.md @@ -18,6 +18,8 @@ azd env set AZURE_INFRASTRUCTURE_SUBNET_ID /subscriptions//resourceGroups/< azd up ``` +After the first deploy, set `allowedOrigins` in `main.parameters.json` to the Container App URL printed by `azd up` (e.g. `https://..eastus.azurecontainerapps.io`), then re-run `azd up` to apply the CORS restriction. + The Bicep template provisions: - A Container Apps managed environment with an internal load balancer (private VNet, no public IP) and a system-assigned managed identity on the Container App (AZR-000363 / AZR-000361 compliant). diff --git a/deploy/azure/main.parameters.json b/deploy/azure/main.parameters.json index 09f51b6a..42c581cb 100644 --- a/deploy/azure/main.parameters.json +++ b/deploy/azure/main.parameters.json @@ -12,7 +12,7 @@ "value": "${SERVICE_EXPLORER_IMAGE_NAME}" }, "allowedOrigins": { - "value": "*" + "value": "https://REPLACE_ME.azurecontainerapps.io" }, "falkordbHost": { "value": "falkordb" diff --git a/deploy/gcp/README.md b/deploy/gcp/README.md index 572f896d..1794628b 100644 --- a/deploy/gcp/README.md +++ b/deploy/gcp/README.md @@ -11,8 +11,9 @@ gcloud builds submit --config deploy/gcp/cloudbuild.yaml \ --substitutions _REGION=us-central1,_SERVICE_NAME=knowledge-explorer,_ALLOWED_ORIGINS=https://knowledge-explorer-REPLACE_ME.a.run.app ``` -For declarative deploys, replace `PROJECT_ID` in `cloudrun-service.yaml`, then run: +For declarative deploys, substitute your project ID and deploy in one step: ```bash -gcloud run services replace deploy/gcp/cloudrun-service.yaml --region us-central1 +sed "s/PROJECT_ID/$(gcloud config get-value project)/g" deploy/gcp/cloudrun-service.yaml | \ + gcloud run services replace - --region us-central1 ``` diff --git a/deploy/gcp/cloudrun-service.yaml b/deploy/gcp/cloudrun-service.yaml index 8fabfb3a..426ea998 100644 --- a/deploy/gcp/cloudrun-service.yaml +++ b/deploy/gcp/cloudrun-service.yaml @@ -20,6 +20,8 @@ spec: timeoutSeconds: 300 containers: - name: explorer + # Replace PROJECT_ID with your GCP project ID before deploying. + # See the README for the sed one-liner that does this automatically. image: gcr.io/PROJECT_ID/knowledge-explorer:latest securityContext: allowPrivilegeEscalation: false diff --git a/deploy/helm/knowledge-explorer/templates/networkpolicy.yaml b/deploy/helm/knowledge-explorer/templates/networkpolicy.yaml index ea022802..07e1928b 100644 --- a/deploy/helm/knowledge-explorer/templates/networkpolicy.yaml +++ b/deploy/helm/knowledge-explorer/templates/networkpolicy.yaml @@ -13,9 +13,30 @@ spec: - Ingress - Egress ingress: - - ports: + # Allow traffic from the ingress controller namespace. + # Override networkPolicy.ingressNamespace in values if your controller uses a different namespace. + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: {{ .Values.networkPolicy.ingressNamespace }} + ports: + - protocol: TCP + port: {{ .Values.service.targetPort }} + # Allow traffic from pods within the same namespace (e.g. monitoring sidecars). + - from: + - podSelector: {} + ports: - protocol: TCP port: {{ .Values.service.targetPort }} egress: - - {} + # FalkorDB + - ports: + - protocol: TCP + port: {{ .Values.networkPolicy.falkordbPort | default 6379 }} + # DNS resolution + - ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 {{- end }} diff --git a/deploy/helm/knowledge-explorer/values.yaml b/deploy/helm/knowledge-explorer/values.yaml index 4004bddf..cfb183a7 100644 --- a/deploy/helm/knowledge-explorer/values.yaml +++ b/deploy/helm/knowledge-explorer/values.yaml @@ -96,3 +96,8 @@ affinity: {} networkPolicy: enabled: true + # Namespace label of your ingress controller. Ingress is only admitted from this namespace + # and from pods within the same namespace as the Explorer. + ingressNamespace: ingress-nginx + # FalkorDB port allowed for egress. Must match FALKORDB_PORT. + falkordbPort: 6379 diff --git a/deploy/kubernetes/deployment.yaml b/deploy/kubernetes/deployment.yaml index abba539d..e9167d66 100644 --- a/deploy/kubernetes/deployment.yaml +++ b/deploy/kubernetes/deployment.yaml @@ -40,6 +40,9 @@ spec: envFrom: - configMapRef: name: knowledge-explorer-config + - secretRef: + name: knowledge-explorer-secrets + optional: true livenessProbe: httpGet: path: /api/health diff --git a/deploy/kubernetes/networkpolicy.yaml b/deploy/kubernetes/networkpolicy.yaml index d4f8ab20..45fc7ebe 100644 --- a/deploy/kubernetes/networkpolicy.yaml +++ b/deploy/kubernetes/networkpolicy.yaml @@ -14,8 +14,29 @@ spec: - Ingress - Egress ingress: - - ports: + # Allow traffic from the ingress controller namespace. + # Adjust the namespace label if your ingress controller uses a different namespace. + - from: + - namespaceSelector: + matchLabels: + kubernetes.io/metadata.name: ingress-nginx + ports: + - protocol: TCP + port: 8000 + # Allow traffic from pods within the same namespace (e.g. monitoring sidecars). + - from: + - podSelector: {} + ports: - protocol: TCP port: 8000 egress: - - {} + # FalkorDB + - ports: + - protocol: TCP + port: 6379 + # DNS resolution + - ports: + - protocol: UDP + port: 53 + - protocol: TCP + port: 53 diff --git a/semantica/explorer/app.py b/semantica/explorer/app.py index 93c9ec54..96d66b14 100644 --- a/semantica/explorer/app.py +++ b/semantica/explorer/app.py @@ -40,12 +40,18 @@ def _read_explorer_settings() -> dict: "allowed_origins": [ origin.strip() for origin in raw_origins.split(",") if origin.strip() ], + # These are read and stored for future use when direct FalkorDB connection + # support is added to the Explorer. Currently GraphSession uses an in-memory + # ContextGraph and does not open a network connection to FalkorDB. "falkordb_host": os.environ.get("FALKORDB_HOST", "localhost"), "falkordb_port": _read_int_env("FALKORDB_PORT", 6379), } def _install_mutation_bridge(app: FastAPI, session: GraphSession) -> None: + if getattr(session.graph, "_mutation_bridge_installed", False): + return + session.graph._mutation_bridge_installed = True previous_callback = getattr(session.graph, "mutation_callback", None) def on_mutation(event_type: str, entity_id: str, payload: dict) -> None: @@ -89,7 +95,6 @@ def create_app(session: Optional[GraphSession] = None) -> FastAPI: ) app.state.explorer_settings = settings - app.state.allowed_origins = settings["allowed_origins"] # allow_credentials lets browsers send cookies/auth headers cross-origin. # The Explorer has no authentication, so credentials serve no purpose and diff --git a/tests/explorer/test_explorer_api.py b/tests/explorer/test_explorer_api.py index a83022b0..18597743 100644 --- a/tests/explorer/test_explorer_api.py +++ b/tests/explorer/test_explorer_api.py @@ -171,7 +171,7 @@ class TestHealthInfo: app = create_app() - assert app.state.allowed_origins == [ + assert app.state.explorer_settings["allowed_origins"] == [ "https://app.example.com", "https://team.example.com", ] @@ -184,7 +184,7 @@ class TestHealthInfo: app = create_app() - assert app.state.allowed_origins == ["https://legacy.example.com"] + assert app.state.explorer_settings["allowed_origins"] == ["https://legacy.example.com"] def test_default_app_initializes_empty_graph_session(self): with TestClient(create_app()) as test_client: