Commit Graph
8 Commits
Author SHA1 Message Date
Mohd KaifandSameer6305 9eea49a070 fix(security): restrict Neptune cookbook SG, add VPC flow logs, harden IaC scan suppressions (#806)
* fix(security): restrict Neptune cookbook SG, add VPC flow logs, harden IaC scan suppressions

Addresses open GHAS code scanning alerts:
- Neptune cookbook stack (neptune-setup.yaml) no longer opens the Bolt/OpenCypher
  port to 0.0.0.0/0; a required ClientCidr parameter must be supplied instead.
  Updated 21_Amazon_Neptune_Store.ipynb deploy instructions to match.
- Added VPC Flow Logs (CloudWatch Logs + IAM role) to the same stack.
- Documented why an account-wide IAM password policy resource does not belong
  in a disposable per-learner CFN stack, with a justified ts:skip.
- Added inline `checkov:skip` / `ts:skip` comments to the knowledge-explorer
  Helm templates (deployment/service/configmap) as a second suppression path
  for the CKV_K8S_21/AC_K8S_0086/AC_K8S_0080 false positives, since the prior
  annotation-only suppression was not being honored by the scanner.

* docs(changelog): document the Neptune and Helm chart security scan fixes

* fix(security): correct flow-log IAM scope and ClientCidr regex from review

- FlowLogRole granted logs:CreateLogStream/PutLogEvents on the bare log
  group ARN, but those actions apply to log streams, not the group itself;
  scoped them to "${FlowLogGroup.Arn}:log-stream:*" instead and moved the
  Describe* actions (which don't support group/stream-level resource
  restriction) to Resource: "*", matching AWS's documented flow-log IAM
  policy shape. Without this, flow log delivery could silently fail.
- ClientCidr's AllowedPattern only checked digit count (1-3 digits per
  octet), so malformed values like 999.999.999.999/32 passed parameter
  validation and would only fail later when CloudFormation tried to
  create the security group rule. Tightened the regex to enforce valid
  IPv4 octet ranges (0-255) and prefix lengths (0-32).

* fix(security): harden IAM policy in neptune-setup and standardize Helm chart scan suppressions

- neptune-setup.yaml: split FlowLogRole policy into account-level statement (CreateLogGroup, DescribeLogGroups, DescribeLogStreams with Resource: '*') and log-group-scoped statement (CreateLogStream, PutLogEvents with !GetAtt FlowLogGroup.Arn) per AWS VPC Flow Logs least-privilege documentation.
- deployment.yaml: remove unreliable file-header skip comments (# checkov:skip / # ts:skip) and replace with resource-level metadata.annotations (checkov.io/skip and runterrascan.io/skip). Update seccomp rule ID from CKV_K8S_28 to checkov's actual seccomp rule CKV_K8S_31 on both Deployment and pod-template metadata.
- configmap.yaml / service.yaml: remove stale # ts:skip=AC_K8S_0086 file-header comments and add runterrascan.io/skip resource-level metadata annotations for consistency across all chart templates.
- .checkov.yaml: update documentation to explain resource-level metadata.annotations and reference CKV_K8S_31.

---------

Co-authored-by: Sameer6305 <sskadam6305@gmail.com>
2026-07-27 17:54:38 +05:30
Sameer6305 2a0bc7051a fix(security): switch to metadata.annotations for CKV_K8S_21 suppressions 2026-07-25 17:45:53 +05:30
KaifAhmad1 ed44260ec3 fix: suppress CKV_K8S_21 false positive on knowledge-explorer Helm chart
Checkov's helm framework renders the chart without a namespace override,
so metadata.namespace (set to .Release.Namespace, bound only at install
time) always resolves to "default" and trips CKV_K8S_21 on service.yaml,
deployment.yaml, and configmap.yaml even though the chart is
namespace-agnostic by design.

Suppressed via per-file checkov:skip comments, following the same
convention already used for the Cloud Run false positives in
deploy/gcp/cloudrun-service.yaml.
2026-07-25 16:49:25 +05:30
KaifAhmad1 2440c5adb4 fix(ci): make .checkov.yaml a valid YAML mapping to prevent NoneType parse error
An empty/comment-only YAML file is parsed as NoneType by PyYAML.
Checkov requires a dict; adding skip-check: [] satisfies the parser
without globally suppressing any checks.
2026-06-24 23:01:28 +05:30
KaifAhmad1 b9e069301f 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
2026-06-24 22:55:18 +05:30
KaifAhmad1 bacc37ab77 fix(ci): suppress CKV_K8S_21 false-positive on Cloud Run Knative YAML
checkov scans deploy/gcp/cloudrun-service.yaml as a Kubernetes resource
because it has apiVersion: serving.knative.dev/v1. It flags CKV_K8S_21
('default namespace should not be used') because Cloud Run services have
no metadata.namespace field — they are project/region scoped, not
namespace scoped. Add CKV_K8S_21 to .checkov.yaml skip-check and to the
inline skip comment in cloudrun-service.yaml.
2026-06-24 14:13:00 +05:30
KaifAhmad1 52e8f38361 fix(ci): move checkov out of MSDO into standalone bridgecrewio/checkov-action
Root cause of 6 consecutive CI failures:
MSDO 0.215.0's guardian.cmd wrapper breaks the build whenever checkov exits
with code 1. Checkov exits 1 on ANY violation, including MEDIUM/LOW findings
that are all 'below minimum severity'. This makes Active results = 0 and
'Found no breaking results', yet Guardian still raises BreakException because
it treats the tool's exit code as a first-class breaking signal. The
.checkov.yaml soft-fail setting was never read because the MSDO runner
bypasses repository config files.

Fix:
- Remove checkov from the MSDO tools list (stops the guardian.cmd crash)
- Add a dedicated 'checkov' job on ubuntu-latest using the official
  bridgecrewio/checkov-action@v12, which runs a current checkov release,
  runs on Linux, and correctly reads .checkov.yaml and respects soft_fail
- Set soft_fail: true in the action so low/medium findings appear in the
  Security tab without ever blocking the build
- MSDO continues to run eslint, templateanalyzer (Bicep/ARM), and terrascan;
  these tools all have well-behaved exit codes and produce no active results
  after the security fixes applied earlier in this PR

.checkov.yaml:
- Replace soft-fail: true (was a failed workaround for MSDO) with
  skip-check: [CKV_K8S_28, CKV_K8S_30] — correct suppression for the
  Knative false-positives (Cloud Run enforces seccomp + AppArmor at
  platform level without requiring K8s annotations)
2026-06-24 14:05:26 +05:30
KaifAhmad1 2f73c1c91d fix(ci): add .checkov.yaml soft-fail to silence tool-error break in MSDO
Active results are 0 and 'Found no breaking results' but MSDO still fails
because checkov exits with code 1 whenever it finds any violation
(including MEDIUM/LOW below the minimum severity threshold). MSDO v1.12.0
treats a non-zero tool exit code as a breaking result even when Guardian
reports no active findings.

soft-fail: true makes checkov exit 0 in all cases. MSDO Guardian still
reads the full SARIF output and would surface any HIGH/CRITICAL findings
as active results that break the build, so the security posture is
unchanged.
2026-06-24 13:31:19 +05:30