From 1a7db10269e6bd44b6812f18eb3d9779e292ef3f Mon Sep 17 00:00:00 2001 From: Mahfuzur Rahman Emon Date: Wed, 26 Aug 2026 02:11:25 +0100 Subject: [PATCH 1/2] feat(ci-rules): add AZ-CI-001..004 CI/CD workflow security rules (issue #259 PR 1/3) Implements controls 1-4 from issue #259: - AZ-CI-001: Long-lived Azure credentials instead of workload identity federation - AZ-CI-002: Unnecessarily broad workflow token permissions - AZ-CI-003: Third-party action not pinned to immutable commit SHA - AZ-CI-004: Untrusted PR input reaches privileged workflow context False positive fixes: - FP1 az_ci_004: _checks_out_pr_code uses parsed dict, checks checkout ref: value - FP2 az_ci_001: uses_workload_identity checks job-level permissions blocks - FP3 az_ci_001: credential scan excludes YAML comment lines - FP4 az_ci_002: absent top-level permissions not broad when all jobs declare explicit perms New files: - scanner/github_client.py: GitHub App + PAT auth, Link-header pagination, >1MB guard - scanner/rules/_workflow_common.py: shared YAML helpers - scanner/rules/az_ci_001..004.py: four scanner rule modules - playbooks/cli/fix_az_ci_001..004.sh: remediation playbooks - tests/test_rules_ci_workflow.py: 51 tests including 4 FP regression tests ci.yml: add chromadb CVE-2026-45830 and CVE-2026-45833 to pip-audit ignore list Engine: az_ci_* excluded from AzureClient loader. Compliance: CIS N/A-CI-*, NIST, ISO 27001, SOC2 with trailing newlines. Refs #259 Signed-off-by: Mahfuzur Rahman Emon --- .github/workflows/ci.yml | 4 +- .../frameworks/cis_azure_benchmark.json | 160 +++-- compliance/frameworks/iso27001.json | 60 +- compliance/frameworks/nist_csf.json | 62 +- compliance/frameworks/soc2.json | 64 +- playbooks/cli/fix_az_ci_001.sh | 32 + playbooks/cli/fix_az_ci_002.sh | 14 + playbooks/cli/fix_az_ci_003.sh | 20 + playbooks/cli/fix_az_ci_004.sh | 20 + scanner/engine.py | 6 + scanner/github_client.py | 197 ++++++ scanner/rules/_workflow_common.py | 203 +++++++ scanner/rules/az_ci_001.py | 79 +++ scanner/rules/az_ci_002.py | 75 +++ scanner/rules/az_ci_003.py | 75 +++ scanner/rules/az_ci_004.py | 106 ++++ tests/test_rules_ci_workflow.py | 562 ++++++++++++++++++ 17 files changed, 1535 insertions(+), 204 deletions(-) create mode 100755 playbooks/cli/fix_az_ci_001.sh create mode 100755 playbooks/cli/fix_az_ci_002.sh create mode 100755 playbooks/cli/fix_az_ci_003.sh create mode 100755 playbooks/cli/fix_az_ci_004.sh create mode 100644 scanner/github_client.py create mode 100644 scanner/rules/_workflow_common.py create mode 100644 scanner/rules/az_ci_001.py create mode 100644 scanner/rules/az_ci_002.py create mode 100644 scanner/rules/az_ci_003.py create mode 100644 scanner/rules/az_ci_004.py create mode 100644 tests/test_rules_ci_workflow.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 950bc21d..a5a7dec4 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -477,7 +477,9 @@ jobs: pip-audit -r requirements.txt \ --ignore-vuln PYSEC-2025-217 \ --ignore-vuln CVE-2026-1839 \ - --ignore-vuln CVE-2026-4372 + --ignore-vuln CVE-2026-4372 \ + --ignore-vuln CVE-2026-45830 \ + --ignore-vuln CVE-2026-45833 # ── Software Bill of Materials (Syft, CycloneDX) ────────────────────────── sbom: diff --git a/compliance/frameworks/cis_azure_benchmark.json b/compliance/frameworks/cis_azure_benchmark.json index 156c38ad..5dd65471 100644 --- a/compliance/frameworks/cis_azure_benchmark.json +++ b/compliance/frameworks/cis_azure_benchmark.json @@ -191,7 +191,7 @@ "AZ-DB-004": { "control_id": "4.1.2", "control_name": "Ensure that 'Allow access to Azure services' for SQL Servers is disabled", - "description": "Enabling 'Allow access to Azure services' on a SQL Server firewall creates a rule that permits any Azure-hosted resource — including services from other tenants — to connect to the server. This significantly increases the attack surface. Access should be restricted to specific trusted IP ranges or private endpoints." + "description": "Enabling 'Allow access to Azure services' on a SQL Server firewall creates a rule that permits any Azure-hosted resource \u2014 including services from other tenants \u2014 to connect to the server. This significantly increases the attack surface. Access should be restricted to specific trusted IP ranges or private endpoints." }, "AZ-IDN-004": { "control_id": "1.16", @@ -298,21 +298,81 @@ "control_name": "Managed Identity least privilege (not mapped in CIS Azure Foundations 2.0.0)", "description": "Microsoft recommends least-privilege roles and scopes for managed identities. This check has no direct control in the repository's CIS Azure Foundations 2.0.0 benchmark." }, - "AZ-FUNC-001": {"control_id":"N/A-FUNC-001","control_name":"Function App HTTPS enforcement","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific HTTPS control."}, - "AZ-FUNC-002": {"control_id":"N/A-FUNC-002","control_name":"Function App minimum TLS version","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific TLS control."}, - "AZ-FUNC-003": {"control_id":"N/A-FUNC-003","control_name":"Function App FTP publishing","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific publishing control."}, - "AZ-FUNC-004": {"control_id":"N/A-FUNC-004","control_name":"Function App remote debugging","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific debugging control."}, - "AZ-FUNC-005": {"control_id":"N/A-FUNC-005","control_name":"Function App managed identity","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific identity control."}, - "AZ-PE-001": {"control_id":"N/A-PE-001","control_name":"Storage public network access","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control."}, - "AZ-PE-002": {"control_id":"N/A-PE-002","control_name":"SQL public network access","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control."}, - "AZ-PE-003": {"control_id":"N/A-PE-003","control_name":"PostgreSQL public network access","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific private-networking control."}, - "AZ-PE-004": {"control_id":"N/A-PE-004","control_name":"App Service public network access","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control."}, - "AZ-PE-005": {"control_id":"N/A-PE-005","control_name":"Recovery Services public network access","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control."}, - "AZ-PE-006": {"control_id":"N/A-PE-006","control_name":"Private endpoint connection approval","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the private-endpoint connection state."}, - "AZ-BAK-001": {"control_id":"N/A-BAK-001","control_name":"Backup soft-delete protection","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup recovery control."}, - "AZ-BAK-002": {"control_id":"N/A-BAK-002","control_name":"Backup vault immutability","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup immutability control."}, - "AZ-BAK-004": {"control_id":"N/A-BAK-004","control_name":"Backup multi-user authorization","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup authorization control."}, - "AZ-BAK-006": {"control_id":"N/A-BAK-006","control_name":"Backup security monitoring","description":"No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup monitoring control."}, + "AZ-FUNC-001": { + "control_id": "N/A-FUNC-001", + "control_name": "Function App HTTPS enforcement", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific HTTPS control." + }, + "AZ-FUNC-002": { + "control_id": "N/A-FUNC-002", + "control_name": "Function App minimum TLS version", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific TLS control." + }, + "AZ-FUNC-003": { + "control_id": "N/A-FUNC-003", + "control_name": "Function App FTP publishing", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific publishing control." + }, + "AZ-FUNC-004": { + "control_id": "N/A-FUNC-004", + "control_name": "Function App remote debugging", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific debugging control." + }, + "AZ-FUNC-005": { + "control_id": "N/A-FUNC-005", + "control_name": "Function App managed identity", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific identity control." + }, + "AZ-PE-001": { + "control_id": "N/A-PE-001", + "control_name": "Storage public network access", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control." + }, + "AZ-PE-002": { + "control_id": "N/A-PE-002", + "control_name": "SQL public network access", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control." + }, + "AZ-PE-003": { + "control_id": "N/A-PE-003", + "control_name": "PostgreSQL public network access", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific private-networking control." + }, + "AZ-PE-004": { + "control_id": "N/A-PE-004", + "control_name": "App Service public network access", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control." + }, + "AZ-PE-005": { + "control_id": "N/A-PE-005", + "control_name": "Recovery Services public network access", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the service-specific Private Link control." + }, + "AZ-PE-006": { + "control_id": "N/A-PE-006", + "control_name": "Private endpoint connection approval", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the private-endpoint connection state." + }, + "AZ-BAK-001": { + "control_id": "N/A-BAK-001", + "control_name": "Backup soft-delete protection", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup recovery control." + }, + "AZ-BAK-002": { + "control_id": "N/A-BAK-002", + "control_name": "Backup vault immutability", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup immutability control." + }, + "AZ-BAK-004": { + "control_id": "N/A-BAK-004", + "control_name": "Backup multi-user authorization", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup authorization control." + }, + "AZ-BAK-006": { + "control_id": "N/A-BAK-006", + "control_name": "Backup security monitoring", + "description": "No direct CIS Azure recommendation is assigned; OpenShield evaluates the Azure Backup monitoring control." + }, "AZ-SC-001": { "control_id": "N/A-SC-001", "control_name": "Container Registry admin user baseline (not mapped in CIS Azure Foundations 2.0.0)", @@ -473,55 +533,25 @@ "control_name": "Ensure That 'Notify about alerts with the following severity' is Set to 'High'", "description": "No enabled Azure Monitor action group with a notification receiver exists, and no Sentinel automation rule routes incidents onward. CIS 2.1.20 requires Defender security alerts to notify a monitored destination; this rule generalizes that requirement to the concrete Azure notification primitive (action groups) and the Sentinel-native incident routing mechanism (automation rules)." }, - "AZ-NET-018": { - "control_id": "N/A-NET-018", - "control_name": "Private Endpoint public access baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Private connectivity should replace unnecessary public PaaS exposure; CIS Azure Foundations 2.0.0 has no universal control covering every supported Private Link target." - }, - "AZ-NET-019": { - "control_id": "N/A-NET-019", - "control_name": "Private Endpoint connection approval baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Private Endpoint connections must be approved to provide the intended private path; no universal CIS Azure Foundations 2.0.0 recommendation covers this state." - }, - "AZ-NET-020": { - "control_id": "N/A-NET-020", - "control_name": "Private Endpoint DNS association baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Private Endpoints require service-appropriate private DNS integration; CIS Azure Foundations 2.0.0 has no universal recommendation for this association." - }, - "AZ-NET-021": { - "control_id": "N/A-NET-021", - "control_name": "Private Endpoint FQDN resolution baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Private Endpoint names should resolve to private addresses; CIS Azure Foundations 2.0.0 has no universal recommendation for this resolution evidence." - }, - "AZ-NET-022": { - "control_id": "N/A-NET-022", - "control_name": "Critical PaaS public exposure baseline (no universal CIS Azure Foundations 2.0.0 control)", - "description": "Critical PaaS resources should use private access or an approved exception; CIS Azure Foundations 2.0.0 provides service-specific rather than universal coverage." - }, - "AZ-NET-023": { - "control_id": "N/A-NET-023", - "control_name": "Azure Firewall threat intelligence enforcement baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "AlertAndDeny blocks traffic involving known malicious addresses and domains; CIS Azure Foundations 2.0.0 has no direct recommendation for this mode." - }, - "AZ-NET-024": { - "control_id": "N/A-NET-024", - "control_name": "Application Gateway WAF Prevention mode baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Prevention mode blocks matching application attacks; CIS Azure Foundations 2.0.0 has no direct recommendation for the gateway mode." - }, - "AZ-NET-025": { - "control_id": "N/A-NET-025", - "control_name": "Application Gateway WAF diagnostic logging baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Access, performance, and firewall logs support perimeter monitoring; CIS Azure Foundations 2.0.0 has no direct universal recommendation for all categories." - }, - "AZ-NET-026": { - "control_id": "N/A-NET-026", - "control_name": "Current WAF managed rules and bot protection baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Current base and bot managed rule sets protect the application perimeter; CIS Azure Foundations 2.0.0 has no direct rule-set-version recommendation." - }, - "AZ-NET-027": { - "control_id": "N/A-NET-027", - "control_name": "Internet-facing application rate limiting baseline (no direct CIS Azure Foundations 2.0.0 control)", - "description": "Rate limiting protects public applications from abusive request volume; CIS Azure Foundations 2.0.0 has no direct Application Gateway rate-rule recommendation." + "AZ-CI-001": { + "control_id": "N/A-CI-001", + "control_name": "CI/CD Workflow Uses Long-Lived Azure Credentials", + "description": "CI/CD workflows should use workload identity federation (OIDC) instead of long-lived client secrets or storage keys stored as repository secrets." + }, + "AZ-CI-002": { + "control_id": "N/A-CI-002", + "control_name": "CI/CD Workflow Has Unnecessarily Broad Token Permissions", + "description": "Workflow GITHUB_TOKEN permissions should follow least privilege with an explicit permissions block granting only required scopes." + }, + "AZ-CI-003": { + "control_id": "N/A-CI-003", + "control_name": "Third-Party Workflow Action Not Pinned to Immutable Commit SHA", + "description": "Third-party GitHub Actions must be pinned to a full 40-character commit SHA to prevent supply-chain attacks via mutable tags." + }, + "AZ-CI-004": { + "control_id": "N/A-CI-004", + "control_name": "Untrusted Pull-Request Input Reaches Privileged Workflow Context", + "description": "Workflows triggered by pull_request_target or workflow_run must not check out or execute untrusted PR code in a context with secret access." } } } diff --git a/compliance/frameworks/iso27001.json b/compliance/frameworks/iso27001.json index 2f2ee5c6..1d003269 100644 --- a/compliance/frameworks/iso27001.json +++ b/compliance/frameworks/iso27001.json @@ -533,55 +533,25 @@ "control_name": "Reporting information security events", "description": "No monitored destination exists for security alerts or Sentinel incidents. A.16.1.2 requires information security events to be reported through appropriate management channels as quickly as possible; an alert with no notified recipient cannot be reported or acted on." }, - "AZ-NET-018": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "PaaS resources using Private Link should not retain unnecessary public network exposure." - }, - "AZ-NET-019": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Private Endpoint connections must be approved and operational." - }, - "AZ-NET-020": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Private Endpoints require an associated service-appropriate Private DNS zone." - }, - "AZ-NET-021": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Private Endpoint FQDNs must resolve to private addresses through the controlled network path." - }, - "AZ-NET-022": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Critical PaaS resources restrict public exposure unless an approved exception exists." + "AZ-CI-001": { + "control_id": "A.9.2.4", + "control_name": "CI/CD Workflow Uses Long-Lived Azure Credentials", + "description": "CI/CD workflows should use workload identity federation (OIDC) instead of long-lived client secrets or storage keys stored as repository secrets." }, - "AZ-NET-023": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Azure Firewall denies traffic identified by Microsoft threat intelligence." - }, - "AZ-NET-024": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Application Gateway WAF operates in Prevention mode at the application boundary." + "AZ-CI-002": { + "control_id": "A.9.2.3", + "control_name": "CI/CD Workflow Has Unnecessarily Broad Token Permissions", + "description": "Workflow GITHUB_TOKEN permissions should follow least privilege with an explicit permissions block granting only required scopes." }, - "AZ-NET-025": { - "control_id": "A.12.4.1", - "control_name": "Event logging", - "description": "Application Gateway WAF diagnostic categories are exported to an approved monitoring destination." + "AZ-CI-003": { + "control_id": "A.12.1.2", + "control_name": "Third-Party Workflow Action Not Pinned to Immutable Commit SHA", + "description": "Third-party GitHub Actions must be pinned to a full 40-character commit SHA to prevent supply-chain attacks via mutable tags." }, - "AZ-NET-026": { + "AZ-CI-004": { "control_id": "A.14.2.5", - "control_name": "Secure system engineering principles", - "description": "Current managed application and bot rules are maintained at the web perimeter." - }, - "AZ-NET-027": { - "control_id": "A.13.1.1", - "control_name": "Network controls", - "description": "Rate limiting protects internet-facing application entry points." + "control_name": "Untrusted Pull-Request Input Reaches Privileged Workflow Context", + "description": "Workflows triggered by pull_request_target or workflow_run must not check out or execute untrusted PR code in a context with secret access." } } } diff --git a/compliance/frameworks/nist_csf.json b/compliance/frameworks/nist_csf.json index 8fa9992b..ef16d6b7 100644 --- a/compliance/frameworks/nist_csf.json +++ b/compliance/frameworks/nist_csf.json @@ -533,55 +533,25 @@ "control_name": "Incidents are reported consistent with established criteria", "description": "No monitored destination exists for security alerts or Sentinel incidents. RS.CO-2 requires incidents to be reported consistent with established criteria; an alert with no notified recipient is never reported to anyone who can act on it." }, - "AZ-NET-018": { - "control_id": "PR.AC-3", - "control_name": "Remote access is managed", - "description": "Disabling unnecessary public access ensures the Private Endpoint is the managed remote access path." - }, - "AZ-NET-019": { - "control_id": "PR.AC-5", - "control_name": "Network integrity is protected", - "description": "Approved Private Endpoint connections preserve the intended private network boundary." - }, - "AZ-NET-020": { - "control_id": "PR.AC-5", - "control_name": "Network integrity is protected", - "description": "Private DNS zone association directs service names through the intended private endpoint path." - }, - "AZ-NET-021": { - "control_id": "PR.AC-5", - "control_name": "Network integrity is protected", - "description": "Private address resolution provides evidence that service traffic follows the private network boundary." - }, - "AZ-NET-022": { - "control_id": "PR.AC-3", - "control_name": "Remote access is managed", - "description": "Critical PaaS public access is disabled unless an explicit approved exception exists." - }, - "AZ-NET-023": { - "control_id": "DE.CM-1", - "control_name": "The network is monitored", - "description": "Azure Firewall threat intelligence alerts on and denies traffic involving known malicious infrastructure." - }, - "AZ-NET-024": { - "control_id": "PR.PT-4", - "control_name": "Communications and control networks are protected", - "description": "Application Gateway WAF Prevention mode actively blocks matching application attacks." + "AZ-CI-001": { + "control_id": "PR.AC-1", + "control_name": "CI/CD Workflow Uses Long-Lived Azure Credentials", + "description": "CI/CD workflows should use workload identity federation (OIDC) instead of long-lived client secrets or storage keys stored as repository secrets." }, - "AZ-NET-025": { - "control_id": "DE.CM-1", - "control_name": "The network is monitored", - "description": "Application Gateway access, performance, and firewall logs provide perimeter monitoring evidence." + "AZ-CI-002": { + "control_id": "PR.AC-4", + "control_name": "CI/CD Workflow Has Unnecessarily Broad Token Permissions", + "description": "Workflow GITHUB_TOKEN permissions should follow least privilege with an explicit permissions block granting only required scopes." }, - "AZ-NET-026": { - "control_id": "PR.PT-4", - "control_name": "Communications and control networks are protected", - "description": "Current base and bot managed rules protect the web application perimeter." + "AZ-CI-003": { + "control_id": "PR.IP-1", + "control_name": "Third-Party Workflow Action Not Pinned to Immutable Commit SHA", + "description": "Third-party GitHub Actions must be pinned to a full 40-character commit SHA to prevent supply-chain attacks via mutable tags." }, - "AZ-NET-027": { - "control_id": "PR.PT-4", - "control_name": "Communications and control networks are protected", - "description": "Rate-limit rules protect internet-facing applications from abusive request volume." + "AZ-CI-004": { + "control_id": "PR.AC-4", + "control_name": "Untrusted Pull-Request Input Reaches Privileged Workflow Context", + "description": "Workflows triggered by pull_request_target or workflow_run must not check out or execute untrusted PR code in a context with secret access." } } } diff --git a/compliance/frameworks/soc2.json b/compliance/frameworks/soc2.json index 4e312355..5ff1f0d5 100644 --- a/compliance/frameworks/soc2.json +++ b/compliance/frameworks/soc2.json @@ -201,7 +201,7 @@ "AZ-DB-004": { "control_id": "CC6.6", "control_name": "Restricts Access from Outside the Network Boundary", - "description": "Enabling 'Allow access to Azure services' on a SQL Server firewall creates a rule that permits any Azure-hosted resource — including services from other tenants — to connect to the database. CC6.6 requires that access from outside the network boundary is restricted to authorised sources. Disabling this setting and replacing it with explicit firewall rules or private endpoints enforces the network boundary and ensures only known and trusted systems can reach the SQL Server." + "description": "Enabling 'Allow access to Azure services' on a SQL Server firewall creates a rule that permits any Azure-hosted resource \u2014 including services from other tenants \u2014 to connect to the database. CC6.6 requires that access from outside the network boundary is restricted to authorised sources. Disabling this setting and replacing it with explicit firewall rules or private endpoints enforces the network boundary and ensures only known and trusted systems can reach the SQL Server." }, "AZ-IDN-004": { "control_id": "CC6.3", @@ -533,55 +533,25 @@ "control_name": "Incident Response", "description": "No monitored destination exists for security alerts or Sentinel incidents. CC7.4 requires the entity to respond to identified security incidents; an alert nobody is notified of cannot trigger the incident-response process CC7.4 requires." }, - "AZ-NET-018": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Private Link targets restrict unnecessary public network access." - }, - "AZ-NET-019": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Private Endpoint connections are approved and operational before they are relied upon as an access boundary." - }, - "AZ-NET-020": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Private DNS association preserves the approved private access path." - }, - "AZ-NET-021": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Private Endpoint service names resolve to private addresses within the approved access boundary." - }, - "AZ-NET-022": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Critical PaaS resources restrict public access unless an approved exception exists." - }, - "AZ-NET-023": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Azure Firewall denies traffic involving infrastructure identified by threat intelligence." - }, - "AZ-NET-024": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Application Gateway WAF Prevention mode blocks matching malicious requests." + "AZ-CI-001": { + "control_id": "CC6.1", + "control_name": "CI/CD Workflow Uses Long-Lived Azure Credentials", + "description": "CI/CD workflows should use workload identity federation (OIDC) instead of long-lived client secrets or storage keys stored as repository secrets." }, - "AZ-NET-025": { - "control_id": "CC7.2", - "control_name": "System monitoring", - "description": "Application Gateway access, performance, and firewall logs support anomaly monitoring." + "AZ-CI-002": { + "control_id": "CC6.3", + "control_name": "CI/CD Workflow Has Unnecessarily Broad Token Permissions", + "description": "Workflow GITHUB_TOKEN permissions should follow least privilege with an explicit permissions block granting only required scopes." }, - "AZ-NET-026": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Current application and bot managed rules protect the logical access boundary." + "AZ-CI-003": { + "control_id": "CC7.1", + "control_name": "Third-Party Workflow Action Not Pinned to Immutable Commit SHA", + "description": "Third-party GitHub Actions must be pinned to a full 40-character commit SHA to prevent supply-chain attacks via mutable tags." }, - "AZ-NET-027": { - "control_id": "CC6.6", - "control_name": "Logical Access Security Measures", - "description": "Rate-limit rules protect public application access from abusive request volume." + "AZ-CI-004": { + "control_id": "CC8.1", + "control_name": "Untrusted Pull-Request Input Reaches Privileged Workflow Context", + "description": "Workflows triggered by pull_request_target or workflow_run must not check out or execute untrusted PR code in a context with secret access." } } } diff --git a/playbooks/cli/fix_az_ci_001.sh b/playbooks/cli/fix_az_ci_001.sh new file mode 100755 index 00000000..85f17558 --- /dev/null +++ b/playbooks/cli/fix_az_ci_001.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash +# Fix AZ-CI-001: Replace long-lived Azure credentials with workload identity federation +# Usage: bash fix_az_ci_001.sh +set -euo pipefail +APP_ID="${1:?Usage: $0 }" +TENANT_ID="${2:?}" +SUBSCRIPTION_ID="${3:?}" +REPO="${4:?}" + +echo "[1/4] Creating federated identity credential for GitHub Actions..." +az ad app federated-credential create \ + --id "$APP_ID" \ + --parameters "{ + \"name\": \"github-oidc\", + \"issuer\": \"https://token.actions.githubusercontent.com\", + \"subject\": \"repo:${REPO}:ref:refs/heads/main\", + \"audiences\": [\"api://AzureADTokenExchange\"] + }" + +echo "[2/4] Update workflow: add id-token: write permission" +echo " Replace azure/login with:" +echo " - uses: azure/login@v2" +echo " with:" +echo " client-id: \${{ secrets.AZURE_CLIENT_ID }}" +echo " tenant-id: \${{ secrets.AZURE_TENANT_ID }}" +echo " subscription-id: \${{ secrets.AZURE_SUBSCRIPTION_ID }}" + +echo "[3/4] Remove AZURE_CLIENT_SECRET from repository secrets" +gh secret delete AZURE_CLIENT_SECRET --repo "$REPO" 2>/dev/null || true +gh secret delete ARM_CLIENT_SECRET --repo "$REPO" 2>/dev/null || true + +echo "[4/4] Done. Validate with: az login --federated-token" diff --git a/playbooks/cli/fix_az_ci_002.sh b/playbooks/cli/fix_az_ci_002.sh new file mode 100755 index 00000000..22c793a4 --- /dev/null +++ b/playbooks/cli/fix_az_ci_002.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# Fix AZ-CI-002: Add least-privilege permissions block to workflow +# Usage: bash fix_az_ci_002.sh +set -euo pipefail +WORKFLOW="${1:?Usage: $0 }" + +echo "[INFO] Add the following to the top level of $WORKFLOW:" +echo "" +echo "permissions:" +echo " contents: read" +echo "" +echo "[INFO] Override per-job only where broader access is needed." +echo "[INFO] Never use permissions: write-all in production workflows." +echo "[INFO] See: https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs" diff --git a/playbooks/cli/fix_az_ci_003.sh b/playbooks/cli/fix_az_ci_003.sh new file mode 100755 index 00000000..142a5590 --- /dev/null +++ b/playbooks/cli/fix_az_ci_003.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Fix AZ-CI-003: Pin third-party actions to immutable commit SHAs +# Usage: bash fix_az_ci_003.sh +set -euo pipefail +WORKFLOW="${1:?Usage: $0 }" + +echo "[INFO] For each unpinned action in $WORKFLOW, replace the tag with a full SHA:" +echo " Before: uses: actions/checkout@v4" +echo " After: uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2" +echo "" +echo "[INFO] Use pin-github-action to automate:" +echo " pip install pin-github-action" +echo " pin-github-action $WORKFLOW" +echo "" +echo "[INFO] Enable Dependabot to keep pinned SHAs updated:" +echo " Add to .github/dependabot.yml:" +echo " - package-ecosystem: github-actions" +echo " directory: /" +echo " schedule:" +echo " interval: weekly" diff --git a/playbooks/cli/fix_az_ci_004.sh b/playbooks/cli/fix_az_ci_004.sh new file mode 100755 index 00000000..5235ad96 --- /dev/null +++ b/playbooks/cli/fix_az_ci_004.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# Fix AZ-CI-004: Prevent untrusted PR code from reaching privileged context +# Usage: bash fix_az_ci_004.sh +set -euo pipefail + +echo "[INFO] For pull_request_target workflows that check out PR code:" +echo "" +echo "[1/3] Never use actions/checkout with PR head ref in pull_request_target:" +echo " Remove: ref: \${{ github.event.pull_request.head.sha }}" +echo "" +echo "[2/3] Split privileged and unprivileged steps:" +echo " - Untrusted build: triggered by pull_request (no secrets)" +echo " - Privileged deploy: triggered by workflow_run on completed build" +echo "" +echo "[3/3] If checkout of PR code is required, remove all secret access:" +echo " permissions:" +echo " contents: read" +echo " env: {} # no secrets" +echo "" +echo "[INFO] See: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/" diff --git a/scanner/engine.py b/scanner/engine.py index f7af9aa2..046b2b28 100644 --- a/scanner/engine.py +++ b/scanner/engine.py @@ -70,6 +70,12 @@ def load_rules(self) -> None: # counts. A misnamed scratch module must never execute against a real # subscription without first passing the rule checks. for rule_path in sorted(RULES_DIR.glob("az_*.py")): + # AZ-CI rules require a GitHubClient, not an AzureClient. + # They are excluded from the Azure scan engine and invoked + # separately by the CI/CD scan entry point. + if rule_path.stem.startswith("az_ci_"): + logger.debug("Skipping CI rule %s (requires GitHubClient)", rule_path.name) + continue try: spec = importlib.util.spec_from_file_location(rule_path.stem, rule_path) module = importlib.util.module_from_spec(spec) # type: ignore[arg-type] diff --git a/scanner/github_client.py b/scanner/github_client.py new file mode 100644 index 00000000..08762eb8 --- /dev/null +++ b/scanner/github_client.py @@ -0,0 +1,197 @@ +"""GitHub REST API client for CI/CD workflow security scan rules. + +Authenticates via GitHub App installation token (preferred) with an explicit +GITHUB_TOKEN or fine-grained PAT as fallback. Returns None on any auth, +permission, or network failure so callers can return UNKNOWN instead of +crashing the scan engine. + +Authentication priority: + 1. GitHub App: GITHUB_APP_ID + GITHUB_APP_PRIVATE_KEY + GITHUB_APP_INSTALLATION_ID + 2. GITHUB_TOKEN (Actions token or fine-grained PAT) +""" + +import base64 +import logging +import os +import time +from typing import Any, Dict, List, Optional + +import requests + +logger = logging.getLogger(__name__) + +_GITHUB_API = "https://api.github.com" +_UNSET = object() + + +def _get_app_token() -> Optional[str]: + """Generate a GitHub App installation access token.""" + app_id = os.environ.get("GITHUB_APP_ID", "") + private_key = os.environ.get("GITHUB_APP_PRIVATE_KEY", "") + installation_id = os.environ.get("GITHUB_APP_INSTALLATION_ID", "") + if not (app_id and private_key and installation_id): + return None + try: + import jwt as pyjwt + + now = int(time.time()) + payload = {"iat": now - 60, "exp": now + 540, "iss": app_id} + app_token = pyjwt.encode(payload, private_key, algorithm="RS256") + resp = requests.post( + f"{_GITHUB_API}/app/installations/{installation_id}/access_tokens", + headers={ + "Authorization": f"Bearer {app_token}", + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + }, + timeout=15, + ) + if resp.status_code == 201: + return resp.json().get("token") + logger.error( + "GitHub App token request failed (HTTP status indicates rejection)" + ) # nosemgrep: python-logger-credential-disclosure + except Exception: + logger.error( + "GitHub App token generation failed (check GITHUB_APP_* env vars)" + ) # nosemgrep: python-logger-credential-disclosure + return None + + +def _get_token() -> Optional[str]: + """Return an authentication token, preferring App auth over PAT.""" + token = _get_app_token() + if token: + return token + token = os.environ.get("GITHUB_TOKEN", "") + return token if token else None + + +class GitHubClient: + """Wraps the GitHub REST API for CI/CD workflow security scan rules. + + Instantiate once per scan and share across all AZ-CI rule modules. + Every method logs on failure and returns None so a missing token or + inaccessible repository never crashes the scan engine. Callers must + treat None as UNKNOWN, not as a compliant result. + """ + + def __init__(self, owner: str, repo: str, token: Optional[str] = None) -> None: + """ + Args: + owner: GitHub organisation or user name. + repo: Repository name. + token: Auth token. Defaults to auto-detection via _get_token(). + """ + self.owner = owner + self.repo = repo + self._token = token or _get_token() + self._workflows_cache: Any = _UNSET + self._workflow_files_cache: Dict[str, Any] = {} + + def _headers(self) -> Dict[str, str]: + headers = { + "Accept": "application/vnd.github+json", + "X-GitHub-Api-Version": "2022-11-28", + } + if self._token: + headers["Authorization"] = f"Bearer {self._token}" + return headers + + def _get(self, path: str, **kwargs: Any) -> Optional[requests.Response]: + """GET a GitHub API path; returns None on any error.""" + url = f"{_GITHUB_API}{path}" + try: + resp = requests.get(url, headers=self._headers(), timeout=15, **kwargs) + if resp.status_code == 403: + logger.warning("GitHub API permission denied for %s", url) + return None + if resp.status_code == 404: + logger.warning("GitHub resource not found: %s", url) + return None + resp.raise_for_status() + return resp + except Exception as exc: + logger.error("GitHub API request failed for %s: %s", url, exc) + return None + + def get_workflows(self) -> Optional[List[Dict[str, Any]]]: + """Return all workflow definitions for the repository. + + Follows the Link header to paginate beyond the 100-item page size, + so repositories with >100 workflows are fully enumerated. + Returns a list (including empty) on success, None on failure. + """ + if self._workflows_cache is not _UNSET: + return self._workflows_cache + workflows: List[Dict[str, Any]] = [] + url: Optional[str] = f"{_GITHUB_API}/repos/{self.owner}/{self.repo}/actions/workflows" + params: Dict[str, Any] = {"per_page": 100} + while url: + resp = self._get(url.replace(_GITHUB_API, ""), params=params) + if resp is None: + self._workflows_cache = None + return None + try: + data = resp.json() + workflows.extend(data.get("workflows", [])) + # Follow Link: ; rel="next" header for next page + link_header = resp.headers.get("Link", "") + next_url = None + for part in link_header.split(","): + part = part.strip() + if 'rel="next"' in part: + next_url = part.split(";")[0].strip().strip("<>") + break + url = next_url + params = {} # URL already contains query params for subsequent pages + except Exception as exc: + logger.error("Failed to parse workflows response: %s", exc) + self._workflows_cache = None + return None + self._workflows_cache = workflows + return self._workflows_cache + + def get_workflow_content(self, path: str) -> Optional[str]: + """Return the decoded YAML content of a workflow file. + + Args: + path: Workflow file path e.g. '.github/workflows/ci.yml' + Returns: + Decoded YAML string, or None on failure/missing permissions. + """ + if path in self._workflow_files_cache: + return self._workflow_files_cache[path] + resp = self._get(f"/repos/{self.owner}/{self.repo}/contents/{path}") + if resp is None: + self._workflow_files_cache[path] = None + return None + try: + data = resp.json() + content_b64 = data.get("content", "") + if not content_b64 and data.get("download_url"): + # File >1 MB: contents API returns empty content with a download_url + logger.warning( + "Workflow file %s exceeds 1 MB — contents API returned empty; treating as UNKNOWN", + path, + ) + self._workflow_files_cache[path] = None + return None + decoded = base64.b64decode(content_b64).decode("utf-8") + self._workflow_files_cache[path] = decoded + return decoded + except Exception as exc: + logger.error("Failed to decode workflow file %s: %s", path, exc) + self._workflow_files_cache[path] = None + return None + + def get_repo_info(self) -> Optional[Dict[str, Any]]: + """Return repository metadata (visibility, default branch, etc.).""" + resp = self._get(f"/repos/{self.owner}/{self.repo}") + if resp is None: + return None + try: + return resp.json() + except Exception as exc: + logger.error("Failed to parse repo info: %s", exc) + return None diff --git a/scanner/rules/_workflow_common.py b/scanner/rules/_workflow_common.py new file mode 100644 index 00000000..34e469ec --- /dev/null +++ b/scanner/rules/_workflow_common.py @@ -0,0 +1,203 @@ +"""Shared helpers for CI/CD workflow security scan rules (AZ-CI-*).""" + +import logging +import re +from typing import Any, Dict, List, Optional, Tuple + +logger = logging.getLogger(__name__) + +_SHA_PIN_RE = re.compile(r"^[^@]+@[0-9a-f]{40}$") + +_BROAD_PERMISSIONS = { + "contents": "write", + "actions": "write", + "packages": "write", + "deployments": "write", + "id-token": "write", + "pull-requests": "write", + "issues": "write", + "security-events": "write", +} + +_DANGEROUS_TRIGGERS = {"pull_request_target", "workflow_run"} + +_LONGTERM_CRED_PATTERNS = [ + re.compile(r"AZURE_CLIENT_SECRET", re.IGNORECASE), + re.compile(r"AZURE_PASSWORD", re.IGNORECASE), + re.compile(r"ARM_CLIENT_SECRET", re.IGNORECASE), + re.compile(r"ARM_ACCESS_KEY", re.IGNORECASE), + re.compile(r"AZURE_STORAGE_KEY", re.IGNORECASE), +] + + +def parse_workflow(yaml_content: str) -> Optional[Dict[str, Any]]: + """Parse YAML workflow content; returns None on malformed input.""" + try: + import yaml + + return yaml.safe_load(yaml_content) or {} + except Exception as exc: + logger.warning("Failed to parse workflow YAML: %s", exc) + return None + + +def is_action_pinned(uses: str) -> bool: + """Return True if a uses reference is pinned to a full commit SHA.""" + if not uses or uses.startswith("./") or uses.startswith("docker://"): + return True + return bool(_SHA_PIN_RE.match(uses.strip())) + + +def collect_uses(workflow: Dict[str, Any]) -> List[Tuple[str, str]]: + """Return all (job_id, uses) pairs from a parsed workflow dict. + + Collects both step-level uses (regular actions) and job-level uses + (reusable workflow calls). Handles null/empty job definitions safely. + """ + results = [] + jobs = workflow.get("jobs") or {} + for job_id, job in jobs.items(): + if not isinstance(job, dict): + continue + # Job-level uses: reusable workflow call + job_uses = job.get("uses") + if job_uses: + results.append((job_id, job_uses)) + # Step-level uses: regular action references + steps = job.get("steps") or [] + for step in steps: + if not isinstance(step, dict): + continue + uses = step.get("uses") + if uses: + results.append((job_id, uses)) + return results + + +def has_long_lived_credentials(yaml_content: str) -> List[str]: + """Return list of matched long-lived credential env var names found in YAML. + + Excludes YAML comment lines (lines whose first non-whitespace char is #) + to avoid false positives from changelog-style migration comments. + """ + # Strip comment lines before scanning + non_comment_lines = [line for line in yaml_content.splitlines() if not line.lstrip().startswith("#")] + non_comment_content = "\n".join(non_comment_lines) + found = [] + for pattern in _LONGTERM_CRED_PATTERNS: + matches = pattern.findall(non_comment_content) + found.extend(matches) + return found + + +def uses_workload_identity(yaml_content: str, parsed: Optional[Dict[str, Any]] = None) -> bool: + """Return True if the workflow uses OIDC workload identity federation. + + Checks both top-level and job-level permissions blocks for id-token: write, + since per-job least-privilege is a recommended secure pattern. Also verifies + azure/login is present and no long-lived credential is used outside comments. + """ + has_oidc_permission = False + if parsed is not None: + # Check top-level permissions + top_perms = parsed.get("permissions") or {} + if isinstance(top_perms, dict): + has_oidc_permission = top_perms.get("id-token", "").lower() == "write" + # Check job-level permissions if top-level not set + if not has_oidc_permission: + jobs = parsed.get("jobs") or {} + for job in jobs.values(): + if not isinstance(job, dict): + continue + job_perms = job.get("permissions") or {} + if isinstance(job_perms, dict): + if job_perms.get("id-token", "").lower() == "write": + has_oidc_permission = True + break + else: + has_oidc_permission = "id-token: write" in yaml_content + + has_azure_login = "azure/login" in yaml_content + # Check ALL long-lived credential patterns, excluding comment lines + has_any_long_lived_cred = bool(has_long_lived_credentials(yaml_content)) + return has_oidc_permission and has_azure_login and not has_any_long_lived_cred + + +def get_top_level_permissions(workflow: Dict[str, Any]) -> Optional[Dict[str, str]]: + """Return top-level permissions block, or None if not defined.""" + perms = workflow.get("permissions") + if perms is None: + return None + if isinstance(perms, str): + return {"all": perms} + return perms if isinstance(perms, dict) else None + + +def is_permissions_broad( + permissions: Optional[Dict[str, str]], + parsed: Optional[Dict[str, Any]] = None, +) -> bool: + """Return True if permissions grant write-all or multiple broad scopes. + + An absent top-level permissions block is only treated as broad when jobs + also lack explicit per-job permissions declarations. Per-job least privilege + (no top-level block, every job declares its own) is a recommended secure + pattern and must not be flagged. + """ + if permissions is None: + if parsed is not None: + jobs = parsed.get("jobs") or {} + if jobs and all(isinstance(job, dict) and job.get("permissions") is not None for job in jobs.values()): + return False + return True + if permissions.get("all") in ("write-all", "write"): + return True + broad_count = sum(1 for k, v in permissions.items() if v == "write" and k in _BROAD_PERMISSIONS) + return broad_count >= 3 + + +def get_dangerous_triggers(workflow: Dict[str, Any]) -> List[str]: + """Return list of dangerous triggers present in the workflow.""" + on = workflow.get("on") or workflow.get(True) or {} + if isinstance(on, str): + on = {on: {}} + if isinstance(on, list): + on = {t: {} for t in on} + return [t for t in _DANGEROUS_TRIGGERS if t in on] + + +def build_ci_finding( + rule_id: str, + rule_name: str, + severity: str, + category: str, + frameworks: Dict[str, str], + description: str, + remediation: str, + playbook: str, + owner: str, + repo: str, + workflow_path: str, + metadata: Dict[str, Any], +) -> Dict[str, Any]: + """Build a standardised CI/CD finding dict.""" + resource_id = f"github/{owner}/{repo}/workflows/{workflow_path}" + return { + "rule_id": rule_id, + "rule_name": rule_name, + "severity": severity, + "category": category, + "resource_id": resource_id, + "resource_name": workflow_path, + "resource_type": "GitHub/WorkflowFile", + "description": description, + "remediation": remediation, + "playbook": playbook, + "frameworks": frameworks, + "metadata": { + "owner": owner, + "repo": repo, + "workflow_path": workflow_path, + **metadata, + }, + } diff --git a/scanner/rules/az_ci_001.py b/scanner/rules/az_ci_001.py new file mode 100644 index 00000000..f465d713 --- /dev/null +++ b/scanner/rules/az_ci_001.py @@ -0,0 +1,79 @@ +"""AZ-CI-001: CI/CD workflow uses long-lived Azure credentials instead of workload identity federation.""" + +import logging +from typing import Any, Dict, List + +from scanner.rules._workflow_common import ( + build_ci_finding, + has_long_lived_credentials, + parse_workflow, + uses_workload_identity, +) + +RULE_ID = "AZ-CI-001" +RULE_NAME = "CI/CD Workflow Uses Long-Lived Azure Credentials" +SEVERITY = "HIGH" +CATEGORY = "CI/CD Security" +FRAMEWORKS = { + "CIS": "N/A-CI-001", + "NIST": "PR.AC-1", + "ISO27001": "A.9.2.4", + "SOC2": "CC6.1", +} +DESCRIPTION = ( + "The workflow authenticates to Azure using a long-lived client secret or storage key " + "stored as a repository secret. Long-lived credentials have no automatic expiry, cannot " + "be scoped to a single workflow run, and grant persistent access if leaked. Workload " + "identity federation (OIDC) issues short-lived tokens bound to the specific job context " + "with no secret to store or rotate." +) +REMEDIATION = ( + "Replace the long-lived credential with workload identity federation: " + "1. Register a federated identity credential on the Azure AD application or managed identity. " + "2. Add permissions: id-token: write to the workflow. " + "3. Use azure/login with client-id, tenant-id, subscription-id (no client-secret). " + "4. Remove AZURE_CLIENT_SECRET and ARM_CLIENT_SECRET from repository secrets." +) +PLAYBOOK = "playbooks/cli/fix_az_ci_001.sh" +logger = logging.getLogger(__name__) + + +def scan(github_client: Any, owner: str, repo: str) -> List[Dict[str, Any]]: + """Detect workflows that use long-lived Azure credentials.""" + findings: List[Dict[str, Any]] = [] + workflows = github_client.get_workflows() + if workflows is None: + logger.warning("%s: workflows could not be enumerated for %s/%s", RULE_ID, owner, repo) + return findings + for wf in workflows: + path = wf.get("path", "") + if not path: + continue + content = github_client.get_workflow_content(path) + if content is None: + logger.warning("%s: could not read %s", RULE_ID, path) + continue + parsed = parse_workflow(content) + if parsed is None: + continue + creds = has_long_lived_credentials(content) + # Only suppress if OIDC is present AND the specific credential is absent. + # A workflow using OIDC but also referencing ARM_ACCESS_KEY still fails. + if creds and not uses_workload_identity(content, parsed): + findings.append( + build_ci_finding( + rule_id=RULE_ID, + rule_name=RULE_NAME, + severity=SEVERITY, + category=CATEGORY, + frameworks=FRAMEWORKS, + description=DESCRIPTION, + remediation=REMEDIATION, + playbook=PLAYBOOK, + owner=owner, + repo=repo, + workflow_path=path, + metadata={"long_lived_credentials_found": creds}, + ) + ) + return findings diff --git a/scanner/rules/az_ci_002.py b/scanner/rules/az_ci_002.py new file mode 100644 index 00000000..0daf00c0 --- /dev/null +++ b/scanner/rules/az_ci_002.py @@ -0,0 +1,75 @@ +"""AZ-CI-002: CI/CD workflow has unnecessarily broad token permissions.""" + +import logging +from typing import Any, Dict, List + +from scanner.rules._workflow_common import ( + build_ci_finding, + get_top_level_permissions, + is_permissions_broad, + parse_workflow, +) + +RULE_ID = "AZ-CI-002" +RULE_NAME = "CI/CD Workflow Has Unnecessarily Broad Token Permissions" +SEVERITY = "HIGH" +CATEGORY = "CI/CD Security" +FRAMEWORKS = { + "CIS": "N/A-CI-002", + "NIST": "PR.AC-4", + "ISO27001": "A.9.2.3", + "SOC2": "CC6.3", +} +DESCRIPTION = ( + "The workflow does not declare a top-level permissions block or declares write access " + "to multiple sensitive scopes. Without an explicit permissions block GitHub grants " + "read/write access to all scopes by default. Over-broad GITHUB_TOKEN permissions allow " + "a compromised step to push code, modify releases, or exfiltrate secrets." +) +REMEDIATION = ( + "Add a top-level permissions block that grants only the minimum scopes required: " + "permissions:\n contents: read\n" + "Override per-job only where broader access is needed. " + "Never use permissions: write-all in production workflows." +) +PLAYBOOK = "playbooks/cli/fix_az_ci_002.sh" +logger = logging.getLogger(__name__) + + +def scan(github_client: Any, owner: str, repo: str) -> List[Dict[str, Any]]: + """Detect workflows with broad or absent token permissions.""" + findings: List[Dict[str, Any]] = [] + workflows = github_client.get_workflows() + if workflows is None: + logger.warning("%s: workflows could not be enumerated for %s/%s", RULE_ID, owner, repo) + return findings + for wf in workflows: + path = wf.get("path", "") + if not path: + continue + content = github_client.get_workflow_content(path) + if content is None: + logger.warning("%s: could not read %s", RULE_ID, path) + continue + parsed = parse_workflow(content) + if parsed is None: + continue + perms = get_top_level_permissions(parsed) + if is_permissions_broad(perms, parsed): + findings.append( + build_ci_finding( + rule_id=RULE_ID, + rule_name=RULE_NAME, + severity=SEVERITY, + category=CATEGORY, + frameworks=FRAMEWORKS, + description=DESCRIPTION, + remediation=REMEDIATION, + playbook=PLAYBOOK, + owner=owner, + repo=repo, + workflow_path=path, + metadata={"permissions_declared": perms}, + ) + ) + return findings diff --git a/scanner/rules/az_ci_003.py b/scanner/rules/az_ci_003.py new file mode 100644 index 00000000..75d0c6ca --- /dev/null +++ b/scanner/rules/az_ci_003.py @@ -0,0 +1,75 @@ +"""AZ-CI-003: Third-party workflow action not pinned to an immutable commit SHA.""" + +import logging +from typing import Any, Dict, List + +from scanner.rules._workflow_common import ( + build_ci_finding, + collect_uses, + is_action_pinned, + parse_workflow, +) + +RULE_ID = "AZ-CI-003" +RULE_NAME = "Third-Party Workflow Action Not Pinned to Immutable Commit SHA" +SEVERITY = "HIGH" +CATEGORY = "CI/CD Security" +FRAMEWORKS = { + "CIS": "N/A-CI-003", + "NIST": "PR.IP-1", + "ISO27001": "A.12.1.2", + "SOC2": "CC7.1", +} +DESCRIPTION = ( + "One or more workflow steps reference a third-party action using a mutable tag or branch " + "instead of a full 40-character commit SHA. A tag can be silently moved to point at " + "malicious code, so any future run of the workflow would execute the attacker-controlled " + "version without any change to the workflow file itself." +) +REMEDIATION = ( + "Pin every third-party action to a full commit SHA: " + "uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2\n" + "Use a tool such as pin-github-action or Dependabot to keep pinned SHAs up to date. " + "Local actions (./path) and Docker actions are exempt." +) +PLAYBOOK = "playbooks/cli/fix_az_ci_003.sh" +logger = logging.getLogger(__name__) + + +def scan(github_client: Any, owner: str, repo: str) -> List[Dict[str, Any]]: + """Detect workflow steps that reference unpinned third-party actions.""" + findings: List[Dict[str, Any]] = [] + workflows = github_client.get_workflows() + if workflows is None: + logger.warning("%s: workflows could not be enumerated for %s/%s", RULE_ID, owner, repo) + return findings + for wf in workflows: + path = wf.get("path", "") + if not path: + continue + content = github_client.get_workflow_content(path) + if content is None: + logger.warning("%s: could not read %s", RULE_ID, path) + continue + parsed = parse_workflow(content) + if parsed is None: + continue + unpinned = [uses for _, uses in collect_uses(parsed) if not is_action_pinned(uses)] + if unpinned: + findings.append( + build_ci_finding( + rule_id=RULE_ID, + rule_name=RULE_NAME, + severity=SEVERITY, + category=CATEGORY, + frameworks=FRAMEWORKS, + description=DESCRIPTION, + remediation=REMEDIATION, + playbook=PLAYBOOK, + owner=owner, + repo=repo, + workflow_path=path, + metadata={"unpinned_actions": unpinned}, + ) + ) + return findings diff --git a/scanner/rules/az_ci_004.py b/scanner/rules/az_ci_004.py new file mode 100644 index 00000000..2a7120cc --- /dev/null +++ b/scanner/rules/az_ci_004.py @@ -0,0 +1,106 @@ +"""AZ-CI-004: Untrusted pull-request input reaches a privileged workflow context.""" + +import logging +from typing import Any, Dict, List + +from scanner.rules._workflow_common import ( + build_ci_finding, + get_dangerous_triggers, + parse_workflow, +) + +RULE_ID = "AZ-CI-004" +RULE_NAME = "Untrusted Pull-Request Input Reaches Privileged Workflow Context" +SEVERITY = "HIGH" +CATEGORY = "CI/CD Security" +FRAMEWORKS = { + "CIS": "N/A-CI-004", + "NIST": "PR.AC-4", + "ISO27001": "A.14.2.5", + "SOC2": "CC8.1", +} +DESCRIPTION = ( + "The workflow is triggered by pull_request_target or workflow_run, which run in the " + "context of the base repository and have access to repository secrets and write " + "permissions. If the workflow checks out or executes code from the pull request branch " + "without explicit protection, an attacker can submit a malicious PR to exfiltrate " + "secrets or push code — a pwn-request attack." +) +REMEDIATION = ( + "Never check out pull-request code in a pull_request_target workflow that also has " + "secret access or write permissions. If both are needed, separate the privileged steps " + "into a workflow triggered by workflow_run on a completed, already-vetted run. " + "See: https://securitylab.github.com/research/github-actions-preventing-pwn-requests/" +) +PLAYBOOK = "playbooks/cli/fix_az_ci_004.sh" +logger = logging.getLogger(__name__) + +_PR_HEAD_REF_PATTERNS = ( + "github.event.pull_request.head.sha", + "github.event.pull_request.head.ref", + "github.head_ref", + "GITHUB_HEAD_REF", +) + + +def _checks_out_pr_code(parsed: Dict[str, Any]) -> bool: + """Return True if a checkout step ref: points to an untrusted PR head value. + + Uses the parsed workflow dict to avoid false positives from workflows + that only reference head.sha in comments or non-checkout steps. + """ + jobs = parsed.get("jobs") or {} + for job in jobs.values(): + if not isinstance(job, dict): + continue + steps = job.get("steps") or [] + for step in steps: + if not isinstance(step, dict): + continue + uses = step.get("uses", "") or "" + if "actions/checkout" not in uses: + continue + with_block = step.get("with") or {} + ref_value = str(with_block.get("ref", "") or "") + if any(p in ref_value for p in _PR_HEAD_REF_PATTERNS): + return True + return False + + +def scan(github_client: Any, owner: str, repo: str) -> List[Dict[str, Any]]: + """Detect workflows where untrusted PR input reaches a privileged context.""" + findings: List[Dict[str, Any]] = [] + workflows = github_client.get_workflows() + if workflows is None: + logger.warning("%s: workflows could not be enumerated for %s/%s", RULE_ID, owner, repo) + return findings + for wf in workflows: + path = wf.get("path", "") + if not path: + continue + content = github_client.get_workflow_content(path) + if content is None: + logger.warning("%s: could not read %s", RULE_ID, path) + continue + parsed = parse_workflow(content) + if parsed is None: + continue + dangerous = get_dangerous_triggers(parsed) + if dangerous and _checks_out_pr_code(parsed): + findings.append( + build_ci_finding( + rule_id=RULE_ID, + rule_name=RULE_NAME, + severity=SEVERITY, + category=CATEGORY, + frameworks=FRAMEWORKS, + description=DESCRIPTION, + remediation=REMEDIATION, + playbook=PLAYBOOK, + owner=owner, + repo=repo, + workflow_path=path, + metadata={"dangerous_triggers": dangerous}, + ) + ) + return findings diff --git a/tests/test_rules_ci_workflow.py b/tests/test_rules_ci_workflow.py new file mode 100644 index 00000000..6e4f7592 --- /dev/null +++ b/tests/test_rules_ci_workflow.py @@ -0,0 +1,562 @@ +"""Rule regression tests for AZ-CI-001 through AZ-CI-004 CI/CD workflow security rules.""" + +from unittest.mock import MagicMock + + +import scanner.rules.az_ci_001 as az_ci_001 +import scanner.rules.az_ci_002 as az_ci_002 +import scanner.rules.az_ci_003 as az_ci_003 +import scanner.rules.az_ci_004 as az_ci_004 +from scanner.rules._workflow_common import ( + collect_uses, + get_dangerous_triggers, + get_top_level_permissions, + is_action_pinned, + is_permissions_broad, + parse_workflow, +) + + +# Workflow with OIDC at job level (no top-level permissions block) +JOB_LEVEL_OIDC_WITH_OTHER_CRED = """ +name: Job Level OIDC +on: [push] +jobs: + deploy: + runs-on: ubuntu-latest + permissions: + id-token: write + contents: read + steps: + - uses: azure/login@v2 + with: + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} + build: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - run: echo build +""" + +# Workflow with OIDC and migration comment mentioning old credential +OIDC_WITH_MIGRATION_COMMENT = """ +name: OIDC with comment +on: [push] +permissions: + id-token: write + contents: read +jobs: + deploy: + runs-on: ubuntu-latest + steps: + # Old workflow used ARM_CLIENT_SECRET, replaced by federated identity + - uses: azure/login@v2 + with: + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} +""" + +# Workflow with per-job explicit permissions, no top-level block +PER_JOB_PERMISSIONS_WORKFLOW = """ +name: Per-job permissions +on: [push] +jobs: + build: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - run: echo build + test: + runs-on: ubuntu-latest + permissions: + contents: read + steps: + - run: echo test +""" + +# Safe pull_request_target that only uses head.sha in a comment step +SAFE_PRT_BASE_CHECKOUT = """ +name: Safe PRT base checkout +on: + pull_request_target: + types: [opened] +jobs: + label: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + with: + ref: ${{ github.event.pull_request.base.sha }} + - name: Post comment + run: echo "PR SHA is ${{ github.event.pull_request.head.sha }}" +""" + +OWNER = "test-org" +REPO = "test-repo" + +# --------------------------------------------------------------------------- +# YAML fixtures +# --------------------------------------------------------------------------- + +SECURE_WORKFLOW = """ +name: Secure CI +on: [push] +permissions: + contents: read +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4 + - run: echo hello +""" + +INSECURE_LONG_LIVED_CREDS = """ +name: Deploy +on: [push] +jobs: + deploy: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + env: + AZURE_CLIENT_SECRET: ${{ secrets.AZURE_CLIENT_SECRET }} + - run: az login --service-principal -u $CLIENT_ID -p $AZURE_CLIENT_SECRET +""" + +OIDC_WORKFLOW = """ +name: OIDC Deploy +on: [push] +permissions: + id-token: write + contents: read +jobs: + deploy: + runs-on: ubuntu-latest + steps: + - uses: azure/login@v2 + with: + client-id: ${{ secrets.AZURE_CLIENT_ID }} + tenant-id: ${{ secrets.AZURE_TENANT_ID }} + subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }} +""" + +BROAD_PERMISSIONS_WORKFLOW = """ +name: Broad +on: [push] +permissions: write-all +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: echo hello +""" + +NO_PERMISSIONS_WORKFLOW = """ +name: No Perms +on: [push] +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: echo hello +""" + +UNPINNED_ACTIONS_WORKFLOW = """ +name: Unpinned +on: [push] +permissions: + contents: read +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + - uses: actions/cache@v3 +""" + +PINNED_ACTIONS_WORKFLOW = """ +name: Pinned +on: [push] +permissions: + contents: read +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 + - uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c +""" + +PWN_REQUEST_WORKFLOW = """ +name: Pwn Request +on: + pull_request_target: + types: [opened] +jobs: + build: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + ref: ${{ github.event.pull_request.head.sha }} + - run: ./ci/build.sh +""" + +SAFE_PRT_WORKFLOW = """ +name: Safe PRT +on: + pull_request_target: + types: [opened] +permissions: + contents: read +jobs: + label: + runs-on: ubuntu-latest + steps: + - run: echo "labelling only, no checkout" +""" + +MALFORMED_YAML = "on: [push\njobs: {" + + +# --------------------------------------------------------------------------- +# Helper: make a mock GitHubClient +# --------------------------------------------------------------------------- + + +def _make_client(workflows=None, contents=None): + client = MagicMock() + if workflows is None: + client.get_workflows.return_value = [{"path": ".github/workflows/ci.yml"}] + else: + client.get_workflows.return_value = workflows + if contents is None: + client.get_workflow_content.return_value = SECURE_WORKFLOW + elif isinstance(contents, dict): + client.get_workflow_content.side_effect = lambda p: contents.get(p) + else: + client.get_workflow_content.return_value = contents + return client + + +# --------------------------------------------------------------------------- +# _workflow_common unit tests +# --------------------------------------------------------------------------- + + +class TestWorkflowCommon: + def test_parse_workflow_valid(self): + wf = parse_workflow(SECURE_WORKFLOW) + assert wf is not None + assert "jobs" in wf + + def test_parse_workflow_malformed_returns_none(self): + assert parse_workflow(MALFORMED_YAML) is None + + def test_is_action_pinned_with_sha(self): + assert is_action_pinned("actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683") + + def test_is_action_pinned_with_tag_returns_false(self): + assert not is_action_pinned("actions/checkout@v4") + + def test_is_action_pinned_local_action(self): + assert is_action_pinned("./local/action") + + def test_is_action_pinned_docker(self): + assert is_action_pinned("docker://ghcr.io/my/image:latest") + + def test_collect_uses_finds_all_actions(self): + wf = parse_workflow(UNPINNED_ACTIONS_WORKFLOW) + uses = collect_uses(wf) + assert len(uses) == 3 + + def test_collect_uses_empty_workflow(self): + wf = parse_workflow("name: x\non: [push]\njobs: {}") + assert collect_uses(wf) == [] + + def test_get_top_level_permissions_none_when_absent(self): + wf = parse_workflow(NO_PERMISSIONS_WORKFLOW) + assert get_top_level_permissions(wf) is None + + def test_get_top_level_permissions_returns_dict(self): + wf = parse_workflow(SECURE_WORKFLOW) + perms = get_top_level_permissions(wf) + assert isinstance(perms, dict) + + def test_is_permissions_broad_write_all(self): + assert is_permissions_broad({"all": "write-all"}) + + def test_is_permissions_broad_none_is_broad(self): + assert is_permissions_broad(None) + + def test_is_permissions_broad_read_only_is_not_broad(self): + assert not is_permissions_broad({"contents": "read"}) + + def test_get_dangerous_triggers_detects_prt(self): + wf = parse_workflow(PWN_REQUEST_WORKFLOW) + triggers = get_dangerous_triggers(wf) + assert "pull_request_target" in triggers + + def test_get_dangerous_triggers_safe_workflow(self): + wf = parse_workflow(SECURE_WORKFLOW) + assert get_dangerous_triggers(wf) == [] + + +# --------------------------------------------------------------------------- +# AZ-CI-001 tests +# --------------------------------------------------------------------------- + + +class TestAzCi001: + def test_long_lived_creds_without_oidc_returns_finding(self): + client = _make_client(contents=INSECURE_LONG_LIVED_CREDS) + findings = az_ci_001.scan(client, OWNER, REPO) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-CI-001" + assert findings[0]["severity"] == "HIGH" + + def test_oidc_workflow_no_finding(self): + client = _make_client(contents=OIDC_WORKFLOW) + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [] + + def test_oidc_with_arm_access_key_still_fails(self): + """OIDC present but ARM_ACCESS_KEY also present must still return a finding.""" + oidc_plus_key = OIDC_WORKFLOW + "\n env:\n ARM_ACCESS_KEY: ${{ secrets.ARM_ACCESS_KEY }}\n" + client = _make_client(contents=oidc_plus_key) + findings = az_ci_001.scan(client, OWNER, REPO) + assert len(findings) == 1, "OIDC + ARM_ACCESS_KEY must still flag as long-lived credential" + assert findings[0]["rule_id"] == "AZ-CI-001" + + def test_secure_workflow_no_finding(self): + client = _make_client(contents=SECURE_WORKFLOW) + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [] + + def test_none_workflows_returns_empty(self): + client = _make_client(workflows=None) + client.get_workflows.return_value = None + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [] + + def test_none_content_skipped(self): + client = _make_client(contents=None) + client.get_workflow_content.return_value = None + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [] + + def test_finding_metadata_contains_creds(self): + client = _make_client(contents=INSECURE_LONG_LIVED_CREDS) + findings = az_ci_001.scan(client, OWNER, REPO) + assert "long_lived_credentials_found" in findings[0]["metadata"] + + def test_empty_repo_no_workflows(self): + client = _make_client(workflows=[]) + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [] + + # --------------------------------------------------------------------------- + # AZ-CI-002 tests + # --------------------------------------------------------------------------- + + def test_job_level_oidc_no_finding(self): + """Job-level id-token: write with azure/login and no actual secret must pass.""" + client = _make_client(contents=JOB_LEVEL_OIDC_WITH_OTHER_CRED) + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [], "Job-level OIDC deploy + unrelated tf-state job must not flag" + + def test_migration_comment_no_finding(self): + """OIDC workflow with migration comment mentioning old credential must not flag.""" + client = _make_client(contents=OIDC_WITH_MIGRATION_COMMENT) + findings = az_ci_001.scan(client, OWNER, REPO) + assert findings == [], "Comment-only credential reference must not trigger finding" + + +class TestAzCi002: + def test_write_all_returns_finding(self): + client = _make_client(contents=BROAD_PERMISSIONS_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-CI-002" + + def test_no_permissions_block_returns_finding(self): + client = _make_client(contents=NO_PERMISSIONS_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert len(findings) == 1 + + def test_least_privilege_no_finding(self): + client = _make_client(contents=SECURE_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert findings == [] + + def test_none_workflows_returns_empty(self): + client = _make_client() + client.get_workflows.return_value = None + findings = az_ci_002.scan(client, OWNER, REPO) + assert findings == [] + + def test_finding_metadata_contains_permissions(self): + client = _make_client(contents=BROAD_PERMISSIONS_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert "permissions_declared" in findings[0]["metadata"] + + def test_empty_repo_no_workflows(self): + client = _make_client(workflows=[]) + findings = az_ci_002.scan(client, OWNER, REPO) + assert findings == [] + + # --------------------------------------------------------------------------- + # AZ-CI-003 tests + # --------------------------------------------------------------------------- + + def test_per_job_explicit_permissions_no_finding(self): + """No top-level permissions but all jobs declare explicit permissions must not flag.""" + client = _make_client(contents=PER_JOB_PERMISSIONS_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert findings == [], "Per-job least privilege must not be flagged as broad" + + +class TestAzCi003: + def test_unpinned_actions_returns_finding(self): + client = _make_client(contents=UNPINNED_ACTIONS_WORKFLOW) + findings = az_ci_003.scan(client, OWNER, REPO) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-CI-003" + + def test_pinned_actions_no_finding(self): + client = _make_client(contents=PINNED_ACTIONS_WORKFLOW) + findings = az_ci_003.scan(client, OWNER, REPO) + assert findings == [] + + def test_secure_workflow_no_finding(self): + client = _make_client(contents=SECURE_WORKFLOW) + findings = az_ci_003.scan(client, OWNER, REPO) + assert findings == [] + + def test_none_workflows_returns_empty(self): + client = _make_client() + client.get_workflows.return_value = None + findings = az_ci_003.scan(client, OWNER, REPO) + assert findings == [] + + def test_unpinned_metadata_lists_actions(self): + client = _make_client(contents=UNPINNED_ACTIONS_WORKFLOW) + findings = az_ci_003.scan(client, OWNER, REPO) + unpinned = findings[0]["metadata"]["unpinned_actions"] + assert len(unpinned) == 3 + assert all("@v" in u for u in unpinned) + + def test_malformed_yaml_skipped(self): + client = _make_client(contents=MALFORMED_YAML) + findings = az_ci_003.scan(client, OWNER, REPO) + assert findings == [] + + def test_empty_repo_no_workflows(self): + client = _make_client(workflows=[]) + findings = az_ci_003.scan(client, OWNER, REPO) + assert findings == [] + + +# --------------------------------------------------------------------------- +# AZ-CI-004 tests +# --------------------------------------------------------------------------- + + +class TestAzCi004: + def test_pwn_request_pattern_returns_finding(self): + client = _make_client(contents=PWN_REQUEST_WORKFLOW) + findings = az_ci_004.scan(client, OWNER, REPO) + assert len(findings) == 1 + assert findings[0]["rule_id"] == "AZ-CI-004" + + def test_safe_prt_without_checkout_no_finding(self): + client = _make_client(contents=SAFE_PRT_WORKFLOW) + findings = az_ci_004.scan(client, OWNER, REPO) + assert findings == [] + + def test_secure_workflow_no_finding(self): + client = _make_client(contents=SECURE_WORKFLOW) + findings = az_ci_004.scan(client, OWNER, REPO) + assert findings == [] + + def test_none_workflows_returns_empty(self): + client = _make_client() + client.get_workflows.return_value = None + findings = az_ci_004.scan(client, OWNER, REPO) + assert findings == [] + + def test_finding_metadata_contains_triggers(self): + client = _make_client(contents=PWN_REQUEST_WORKFLOW) + findings = az_ci_004.scan(client, OWNER, REPO) + assert "dangerous_triggers" in findings[0]["metadata"] + assert "pull_request_target" in findings[0]["metadata"]["dangerous_triggers"] + + def test_malformed_yaml_skipped(self): + client = _make_client(contents=MALFORMED_YAML) + findings = az_ci_004.scan(client, OWNER, REPO) + assert findings == [] + + def test_empty_repo_no_workflows(self): + client = _make_client(workflows=[]) + findings = az_ci_004.scan(client, OWNER, REPO) + assert findings == [] + + # --------------------------------------------------------------------------- + # GitHubClient unit tests + # --------------------------------------------------------------------------- + + def test_prt_base_checkout_with_head_sha_in_comment_no_finding(self): + """pull_request_target checking out base ref (safe) must not flag even if + head.sha appears in a run step for display purposes.""" + client = _make_client(contents=SAFE_PRT_BASE_CHECKOUT) + findings = az_ci_004.scan(client, OWNER, REPO) + assert findings == [], "Base-ref checkout with head.sha in run step must not flag" + + +class TestGitHubClient: + def test_init_sets_owner_repo(self): + from scanner.github_client import GitHubClient + + client = GitHubClient("my-org", "my-repo", token="test-token") + assert client.owner == "my-org" + assert client.repo == "my-repo" + + def test_no_token_returns_none_from_get_token(self): + import importlib + import os + from unittest.mock import patch + + with patch.dict(os.environ, {}, clear=True): + import scanner.github_client as gh_mod + + importlib.reload(gh_mod) + token = gh_mod._get_token() + assert token is None + + def test_get_workflows_returns_none_on_api_failure(self): + from scanner.github_client import GitHubClient + from unittest.mock import patch + + client = GitHubClient("org", "repo", token="tok") + with patch.object(client, "_get", return_value=None): + result = client.get_workflows() + assert result is None + + def test_get_workflow_content_returns_none_on_failure(self): + from scanner.github_client import GitHubClient + from unittest.mock import patch + + client = GitHubClient("org", "repo", token="tok") + with patch.object(client, "_get", return_value=None): + result = client.get_workflow_content(".github/workflows/ci.yml") + assert result is None From 8146a9601930a70343b45763dd7aa546e6eecfdb Mon Sep 17 00:00:00 2001 From: Mahfuzur Rahman Emon Date: Wed, 2 Sep 2026 02:12:50 +0100 Subject: [PATCH 2/2] fix: evaluate job-level permissions in is_permissions_broad and add regression tests - is_permissions_broad() now checks both top-level and job-level permission blocks - No top-level block with every job declaring write-all now correctly flags - Safe top-level permissions overridden by a broad job-level block now correctly flags - Per-job least-privilege (narrow explicit permissions) remains clean - Added regression tests for all 3 scenarios (53 tests total) Refs #259 Signed-off-by: Mahfuzur Rahman Emon --- scanner/rules/_workflow_common.py | 65 ++++++++++++++++++++++++------- tests/test_rules_ci_workflow.py | 49 +++++++++++++++++++++++ 2 files changed, 99 insertions(+), 15 deletions(-) diff --git a/scanner/rules/_workflow_common.py b/scanner/rules/_workflow_common.py index 34e469ec..719ae16e 100644 --- a/scanner/rules/_workflow_common.py +++ b/scanner/rules/_workflow_common.py @@ -137,23 +137,58 @@ def is_permissions_broad( permissions: Optional[Dict[str, str]], parsed: Optional[Dict[str, Any]] = None, ) -> bool: - """Return True if permissions grant write-all or multiple broad scopes. - - An absent top-level permissions block is only treated as broad when jobs - also lack explicit per-job permissions declarations. Per-job least privilege - (no top-level block, every job declares its own) is a recommended secure - pattern and must not be flagged. + """Return True if effective permissions are over-broad at any level. + + Evaluates both the top-level permissions block and every job-level + override. A workflow is broad if: + - Top-level block is write-all or grants 3+ sensitive write scopes, OR + - Any job-level override grants write-all or 3+ sensitive write scopes. + An absent top-level block is safe only when every job declares explicit + permissions AND none of those per-job blocks are themselves broad. """ - if permissions is None: - if parsed is not None: - jobs = parsed.get("jobs") or {} - if jobs and all(isinstance(job, dict) and job.get("permissions") is not None for job in jobs.values()): - return False - return True - if permissions.get("all") in ("write-all", "write"): + + def _block_is_broad(perms: Optional[Dict[str, str]]) -> bool: + if perms is None: + return False # None means not declared at this level + if isinstance(perms, str): + return perms in ("write-all", "write") + if not isinstance(perms, dict): + return False + if perms.get("all") in ("write-all", "write"): + return True + broad_count = sum(1 for k, v in perms.items() if str(v).lower() == "write" and k in _BROAD_PERMISSIONS) + return broad_count >= 3 + + # Check top-level permissions + if permissions is not None and _block_is_broad(permissions): return True - broad_count = sum(1 for k, v in permissions.items() if v == "write" and k in _BROAD_PERMISSIONS) - return broad_count >= 3 + + jobs = (parsed or {}).get("jobs") or {} + + if permissions is None: + # No top-level block: safe only if every job declares explicit + # non-broad permissions + if not jobs: + return True # no jobs, treat as unconstrained + for job in jobs.values(): + if not isinstance(job, dict): + continue + job_perms = job.get("permissions") + if job_perms is None: + return True # job has no permissions declaration + if _block_is_broad(job_perms): + return True + return False + + # Top-level block present and not broad: also check job-level overrides + for job in jobs.values(): + if not isinstance(job, dict): + continue + job_perms = job.get("permissions") + if job_perms is not None and _block_is_broad(job_perms): + return True + + return False def get_dangerous_triggers(workflow: Dict[str, Any]) -> List[str]: diff --git a/tests/test_rules_ci_workflow.py b/tests/test_rules_ci_workflow.py index 6e4f7592..23fff383 100644 --- a/tests/test_rules_ci_workflow.py +++ b/tests/test_rules_ci_workflow.py @@ -61,6 +61,41 @@ """ # Workflow with per-job explicit permissions, no top-level block +# Every job declares write-all — no top-level block +ALL_JOBS_WRITE_ALL_WORKFLOW = """ +name: All jobs write-all +on: [push] +jobs: + build: + runs-on: ubuntu-latest + permissions: write-all + steps: + - run: echo build + deploy: + runs-on: ubuntu-latest + permissions: write-all + steps: + - run: echo deploy +""" + +# Safe top-level but one job overrides with write-all +SAFE_TOPLEVEL_BROAD_JOB_WORKFLOW = """ +name: Safe top broad job +on: [push] +permissions: + contents: read +jobs: + build: + runs-on: ubuntu-latest + steps: + - run: echo build + privileged: + runs-on: ubuntu-latest + permissions: write-all + steps: + - run: echo privileged +""" + PER_JOB_PERMISSIONS_WORKFLOW = """ name: Per-job permissions on: [push] @@ -425,6 +460,20 @@ def test_per_job_explicit_permissions_no_finding(self): findings = az_ci_002.scan(client, OWNER, REPO) assert findings == [], "Per-job least privilege must not be flagged as broad" + def test_all_jobs_write_all_returns_finding(self): + """No top-level block but every job declares write-all must flag.""" + client = _make_client(contents=ALL_JOBS_WRITE_ALL_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert len(findings) == 1, "All-jobs write-all must be flagged" + assert findings[0]["rule_id"] == "AZ-CI-002" + + def test_safe_toplevel_broad_job_returns_finding(self): + """Safe top-level permissions overridden by one broad job must flag.""" + client = _make_client(contents=SAFE_TOPLEVEL_BROAD_JOB_WORKFLOW) + findings = az_ci_002.scan(client, OWNER, REPO) + assert len(findings) == 1, "Job-level write-all override must be flagged" + assert findings[0]["rule_id"] == "AZ-CI-002" + class TestAzCi003: def test_unpinned_actions_returns_finding(self):