Skip to content

Add enterprise AKS and workload security controls - #293

Draft
ritiksah141 wants to merge 4 commits into
devfrom
feat/aks-container-controls-255
Draft

Add enterprise AKS and workload security controls#293
ritiksah141 wants to merge 4 commits into
devfrom
feat/aks-container-controls-255

Conversation

@ritiksah141

@ritiksah141 ritiksah141 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Summary

Implements the complete enterprise AKS and container workload security backlog from issue #255 with fifteen evidence-rich controls covering API exposure, Kubernetes network policy, Defender for Containers, secret protection, privileged and host access, cluster administration, and image supply chain policy.

What changed

  • Added AZ-AKS-007 to require a private API server or organization-approved authorized IP ranges.
  • Added AZ-AKS-008 to require Azure, Calico, or Cilium network policy enforcement.
  • Added AZ-AKS-009 to detect eligible namespaces without a NetworkPolicy.
  • Added AZ-AKS-010 to require Microsoft Defender for Containers.
  • Added AZ-AKS-011 to detect native Kubernetes Secret references without Azure Key Vault KMS protection.
  • Added AZ-AKS-012 to require automatic rotation when the Key Vault Secrets Store CSI provider is enabled.
  • Added AZ-AKS-013 through AZ-AKS-017 as separate controls for privileged containers, hostNetwork, hostPID, hostIPC, and hostPath volumes.
  • Added AZ-AKS-018 to detect cluster-admin bindings outside the approved subject allowlist.
  • Added AZ-AKS-019 through AZ-AKS-021 as separate controls for untrusted registries, latest or implicit latest tags, and mutable non-digest image references.
  • Added a cached AKS evidence collector for ARM, Defender for Cloud, Kubernetes namespaces, workloads, NetworkPolicies, and ClusterRoleBindings.
  • Added strict policy loading through OPENSHIELD_AKS_SECURITY_POLICY and Kubernetes access through OPENSHIELD_AKS_KUBECONFIG.
  • Added cluster, namespace, workload, container, image, subject, role, evidence, remediation, permissions, severity, confidence, and unknown reason metadata.
  • Added an individual review-gated remediation playbook for every new rule.
  • Added complete CIS, NIST CSF, ISO 27001, and SOC 2 mappings with explicit reviewed N/A-* CIS decisions where no direct benchmark mapping exists.
  • Updated backend documentation, rule references, architecture guidance, contributor guidance, and the changelog without modifying /website.

False-finding safeguards

  • Empty AKS inventory is treated as not applicable.
  • Missing policy, malformed policy, unavailable credentials, unauthorized Kubernetes access, and API failures are indeterminate and never create findings.
  • Stopped clusters and unsupported provisioning states are reported as unknown rather than pass or fail.
  • Failed namespace discovery is explicitly unknown, while namespace-specific failures are preserved as partial evidence.
  • Partial collection retains valid positive evidence but never infers a missing NetworkPolicy for a namespace that could not be fully inspected.
  • Private clusters satisfy the API exposure control without requiring public authorized IP ranges.
  • Authorized IP ranges must be contained by an approved policy range, and malformed address evidence remains unknown.
  • System and policy-excluded namespaces are omitted from workload and namespace policy findings.
  • CSI rotation is not applicable when the CSI provider is disabled.
  • Native Secret findings require observed workload references and do not infer secret use from cluster configuration alone.
  • Controller-owned Pods are excluded to prevent duplicate findings already represented by their workload controller.
  • Registry, cluster-admin, and namespace exceptions require explicit normalized policy entries.

Verification

  • Focused AKS collector, evaluator, Azure client, and policy suite: 94 passed.
  • Full backend suite: 819 passed, 2 skipped.
  • Total backend coverage: 86.85 percent, above the required 80 percent.
  • Ruff check and formatting validation pass across the repository.
  • Bandit reports no medium or high findings.
  • Semgrep security-audit, OWASP Top Ten, Python, and JavaScript scans report no findings.
  • Pip Audit reports no known vulnerabilities after the repository's existing tracked CI exclusions.
  • Gitleaks reports no secrets in the staged change.
  • Python package dependency validation reports no broken requirements.
  • Compliance JSON and the fifteen-rule mapping matrix validate successfully.
  • All sixteen AKS playbook scripts pass shell syntax validation.
  • Git diff whitespace validation passes.
  • Repository pre-commit checks pass.
  • DCO verification passes for the commit.

Type of change

  • New scan rules
  • Azure and Kubernetes evidence collectors
  • Remediation playbooks
  • Documentation
  • Compliance mappings
  • Regression and failure-path tests

Testing

  • Tested against a real Azure subscription and AKS cluster
  • Positive and secure evaluation paths covered
  • Empty inventory and not-applicable behavior covered
  • Stopped, unsupported, unauthorized, and unreachable states covered
  • Missing and malformed policy covered
  • Partial and incomplete namespace discovery covered
  • Local CI-equivalent backend and security checks pass
  • No hardcoded credentials or secrets

Checklist

  • All fifteen issue Implement enterprise AKS and container workload security rules #255 controls have collection, evaluation, evidence, remediation, mappings, and tests
  • Every rule has its own executable playbook
  • API and permission failures never become false PASS or FAIL results
  • Every commit includes a DCO Signed-off-by trailer
  • All four required compliance frameworks include AZ-AKS-007 through AZ-AKS-021
  • No unresolved TBD-* framework mappings were introduced
  • Branch was created from the latest origin dev
  • Branch name follows the project convention

Related issue

Closes #255

Files to review

  1. scanner/aks_security.py and scanner/azure_client.py - failure-aware ARM, Defender, and Kubernetes evidence collection.
  2. scanner/rules/_aks_enterprise_common.py and scanner/rules/az_aks_007.py through scanner/rules/az_aks_021.py - strict policy evaluation and the fifteen independent controls.
  3. tests/test_aks_security_collector.py, tests/test_rules_aks_enterprise.py, and tests/test_azure_client_aks.py - positive, secure, empty, malformed, partial, unsupported, and failure coverage.
  4. playbooks/cli/fix_az_aks_007.sh through playbooks/cli/fix_az_aks_021.sh - individual review-gated remediation entry points.
  5. compliance/frameworks/*.json - verified CIS, NIST CSF, ISO 27001, and SOC 2 mappings.
  6. config/aks-security-policy.example.json, docs/aks-security-rules.md, docs/rules-reference.md, and CHANGELOG.md - policy schema, behavior, permissions, limitations, and release documentation.

Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
@github-actions

Copy link
Copy Markdown

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails
pip/kubernetes 36.0.0 🟢 6.2
Details
CheckScoreReason
Maintained🟢 1030 commit(s) and 4 issue activity found in the last 90 days -- score normalized to 10
Code-Review🟢 10all changesets reviewed
Packaging⚠️ -1packaging workflow not detected
Dangerous-Workflow🟢 10no dangerous workflow patterns detected
CII-Best-Practices⚠️ 0no effort to earn an OpenSSF best practices badge detected
Token-Permissions⚠️ 0detected GitHub workflow tokens with excessive permissions
License🟢 10license file detected
Signed-Releases⚠️ -1no releases found
Branch-Protection⚠️ -1internal error: error during branchesHandler.setup: internal error: some github tokens can't read classic branch protection rules: https://github.com/ossf/scorecard-action/blob/main/docs/authentication/fine-grained-auth-token.md
Security-Policy🟢 10security policy file detected
SAST⚠️ 0SAST tool is not run on all commits -- score normalized to 0
Binary-Artifacts🟢 10no binaries found in the repo
Fuzzing⚠️ 0project is not fuzzed
Pinned-Dependencies⚠️ 0dependency not pinned by hash detected -- score normalized to 0

Scanned Files

  • requirements.txt

@ritiksah141 ritiksah141 self-assigned this Aug 20, 2026
@ritiksah141
ritiksah141 marked this pull request as ready for review August 20, 2026 17:52

@TFT444 TFT444 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good structure and solid failure-path coverage. Two blockers and two nits before merge.

1. policy_required set too broad (blocker)

scanner/rules/_aks_enterprise_common.py:197

Eight controls (privileged, host_network, host_pid, host_ipc, host_path, secret_protection, network_policy_namespaces, latest_image) return [] when OPENSHIELD_AKS_SECURITY_POLICY is not set, even though none of them need org-specific values. Each already handles policy=None safely downstream. Only four controls genuinely need a configured policy: api_restrictions, cluster_admin, untrusted_registry, and mutable_image. Remove the other eight from the set so workload rules run without a policy file.

2. Docker Hub short-form images cause false-positive AZ-AKS-019 findings (blocker)

scanner/rules/_aks_enterprise_common.py:1581

Images without a registry hostname (nginx, redis:7) fail the trusted-registry check even when docker.io/ is in trusted_registry_prefixes, because nginx.startswith(docker.io/) is False. Normalize short-form names before the prefix match.

3. Approved IP list rebuilt per cluster (efficiency)

scanner/rules/_aks_enterprise_common.py:1400

approved = [ipaddress.ip_network(item) for item in policy.approved_authorized_ip_ranges] is inside the per-cluster loop. Hoist it above for evidence in evidence_items.

4. Stale rule count in docs/learn/index.html (trivial)

Heading says 95 Azure security rules but the paragraph body still reads 80 dynamic rules.

Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
@ritiksah141

Copy link
Copy Markdown
Collaborator Author

Addressed the requested code changes and added regression coverage. I left the docs/learn/index.html rule count unchanged because the assignee handling docs/learn will update it.

@TFT444

TFT444 commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Addressed the requested code changes and added regression coverage. I left the docs/learn/index.html rule count unchanged because the assignee handling docs/learn will update it.

cool make sence now its good to go for me approving it

TFT444
TFT444 previously approved these changes Aug 20, 2026

@m-khan-97 m-khan-97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking on this AKS security pack. There is a lot of good structure here, especially around preserving unknown and partial evidence. Before we merge it, I need us to close three gaps that could produce incorrect results in a real multi-cluster environment:

  1. Cluster evidence can be attached to the wrong AKS resource. The collector selects kubeconfig contexts using only cluster.name. AKS names are scoped to a resource group, so two clusters in one subscription can share a name, and kubeconfig context names can also be customized. In that situation, both ARM resources may be evaluated through the same Kubernetes context. Please introduce an explicit, unambiguous resource-ID-to-context mapping, validate it, and treat missing or ambiguous mappings as UNKNOWN. Add a regression test with duplicate cluster names in different resource groups.

  2. The cluster-admin allowlist loses the ServiceAccount namespace. ServiceAccounts are namespace-scoped, but AZ-AKS-018 compares only Kind:name. Allowing ServiceAccount:builder would therefore allow a builder account from every namespace. Please use a namespace-aware identity such as ServiceAccount:<namespace>:<name>, reject ambiguous policy entries, and test two same-named ServiceAccounts in different namespaces. Group and User identities can keep the appropriate non-namespaced form.

  3. Projected Secret volumes are not collected. _workload() checks ordinary Secret volumes and environment references, but misses volume.projected.sources[].secret.name. A workload using only a projected Secret can therefore be treated as not applicable when KMS is disabled. Please collect those references and add a regression test.

The GitHub checks are green. My focused local run could not collect because this checkout is missing azure.mgmt.authorization; that environment limitation is separate from the production evidence cases above.

Please let me know once these cases are covered and I will re-review promptly.

Signed-off-by: ritiksah141 <ritiksah141@gmail.com>
@ritiksah141

ritiksah141 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@m-khan-97, Addressed the tenant-scale review blockers in 9d6acb2. AKS contexts now use a validated resource-ID mapping, ServiceAccount allowlists are namespace-scoped, and projected Secret volumes are collected. Added regression coverage for each case. All local tests and vulnerability checks pass.

@ritiksah141

ritiksah141 commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator Author

@Vishnu2707, @m-khan-97, addressed the trusted-registry boundary issue in 98ff7da.

Registry trust now respects repository boundaries, so contoso.azurecr.io/team trusts images under that repository but does not trust a lookalike such as contoso.azurecr.io/team-evil. I also added regression coverage for both the trusted and untrusted cases.

Verification: 98 AKS enterprise rule tests passed, with Ruff and formatting checks clean.

I agree that the remaining items are project-wide dependencies rather than PR-specific defects. I’ll keep #293 open and rebase it after #263 establishes explicit UNKNOWN semantics and #308 lands the CRITICAL severity scoring contract. So, for now this PR is in draft and will be updated when its co-revelant parts land on the codebase.

Signed-off-by: ritiksah141 <ritiksah141@gmail.com>

@parthrohit22 parthrohit22 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deep-reviewed _aks_enterprise_common.py (the shared evaluator all 15 new rules dispatch into), aks_security.py's collector, the azure_client.py diff, and traced logic through az_aks_007/009/011/013/018/019/020/021. This is a strong PR overall: the indeterminate-handling invariant is correctly applied across every control branch I walked (missing/None evidence, malformed IP ranges, malformed cluster-admin subjects, UNKNOWN status, missing/malformed policy all skip rather than assert compliance or a confirmed HIGH), the trusted-registry-prefix boundary bug this codebase apparently hit before (per the commit history) is correctly avoided here, RULE_IDs don't collide with anything existing, all 15 playbooks exist/pass bash -n and route through a shared confirmation-gated script that only auto-applies for the 4 rules with unambiguous low-blast-radius remediation, and framework mappings are complete and valid across all four compliance JSONs.

One real bug though: excluded_namespaces from the operator's AKS security policy is silently ignored by 8 of the 15 new rules, contradicting this PR's own documented safeguard. Verified with a standalone repro, not just by reading: built an AksClusterEvidence with excluded_namespaces: ["istio-system"] and a privileged workload with no NetworkPolicy in that namespace - az_aks_009.scan() and az_aks_013.scan() both still emit findings for it despite the explicit exclusion. Left an inline comment with the specific lines and root cause.

Why this got past the existing tests: every fixture that sets excluded_namespaces uses ["kube-system"], which is already unconditionally filtered at the collector level regardless of policy (SYSTEM_NAMESPACES) - so those tests pass whether or not the policy-based exclusion path actually runs. There's no test anywhere with a non-system excluded namespace for any of the 8 affected rules.

Concrete impact: an operator adds a service-mesh control-plane namespace (istio-system, linkerd) or a vendor-managed namespace to excluded_namespaces, per docs/aks-security-rules.md's own description of that field - findings keep firing on it anyway across network_policy_namespaces, secret_protection, privileged, host_network/pid/ipc/path, and latest_image. It's noise rather than a missed real risk (doesn't violate the "ambiguous evidence never confirmed HIGH" invariant), but it's broad and it's a documented claim that doesn't hold, which will train operators to distrust the tool's output on exactly the namespaces they explicitly told it to ignore.

Two non-blocking notes:

  • _normalize_image_registry maps bare refs like nginx:latest to docker.io/nginx:latest, not Docker's canonical docker.io/library/nginx:latest. Internally consistent (tested), not a bug as long as operators write docker.io/ in their policy - but a policy author who writes docker.io/library/ per Docker's own convention would be surprised when official-image workloads keep getting flagged. Worth a one-line callout in the docs or the example policy comment.
  • _collect_kubernetes's six workload-kind calls per namespace (Deployment/StatefulSet/DaemonSet/Job/CronJob/Pod) share one try/except, so an early failure (e.g. RBAC denies listing DaemonSets) aborts the remaining calls for that namespace and marks it wholly partial, even if later calls would have succeeded. Conservative in the safe direction (under-reports, never a false HIGH), so not flagging as a bug - just noting it's untested at the collector level and slightly reduces detection completeness under partial RBAC.

Ran the full suite after installing the kubernetes==36.0.0 dependency this PR adds (missing from this sandbox's .venv, an environment gap worth flagging in case CI needs it fresh too): 822 passed, 3 skipped, 0 failed. Test coverage for the new rules is substantive - compliant/noncompliant paths, empty inventory, inventory failure, unreachable cluster, malformed policy, missing policy split by policy-required-vs-independent rule sets, partial collection never inferring a missing NetworkPolicy, ServiceAccount-scoped cluster-admin allowlisting, IP-supernet containment, and registry prefix-boundary crossing are all exercised.

if not evidence_items:
logger.info("%s: no AKS clusters exist; result is NOT_APPLICABLE", rule_id)
return []
policy_required = control in {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

policy_required only covers api_restrictions/cluster_admin/untrusted_registry/mutable_image, so policy stays None for every other control - network_policy_namespaces, secret_protection, privileged, host_network/host_pid/host_ipc/host_path, and latest_image. That means _workloads(evidence, policy) at line ~189 (if policy and namespace.lower() in policy.excluded_namespaces) and the network_policy_namespaces branch's excluded = policy.excluded_namespaces if policy else frozenset() around line 387 can never apply the operator's configured namespace exclusions for those 8 rules, even though docs/aks-security-rules.md documents excluded_namespaces as a general policy field with no such caveat, and the PR description states "System and policy-excluded namespaces are omitted from workload and namespace policy findings" without qualification.

Suggest loading the policy for namespace-exclusion purposes independent of policy_required - e.g. always call policy_from_env(rule_id) and only gate the stricter "policy is mandatory to run this control at all" behavior behind policy_required, so namespace exclusion applies uniformly regardless of which control is running.

@ritiksah141
ritiksah141 marked this pull request as draft August 24, 2026 16:01
@m-khan-97

Copy link
Copy Markdown
Collaborator

@ritiksah141, Parth's namespace-exclusion finding is valid. Please update the shared evaluator so OPENSHIELD_AKS_SECURITY_POLICY is loaded when present for every control, while only the four policy-required controls fail closed when it is absent or invalid. That allows excluded_namespaces to apply consistently without making the policy-independent rules require configuration.\n\nPlease add regression coverage with a non-system namespace such as istio-system across the affected paths, including AZ-AKS-009 and at least the shared workload path used by AZ-AKS-011/013-017/020. Keep #293 draft while this is addressed and while #263/#308 remain dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement enterprise AKS and container workload security rules

4 participants