Security updates (#188)

Author: simonkurtz-MSFT

.github/workflows/dependency-review.yml

@@ -26,11 +26,19 @@ jobs:
           # ISC: pexpect, ptyprocess
           # LGPL-3.0-or-later: pyzmq (libzmq shared library)
           # HPND: pillow
+          # Python-2.0, GPL-1.0-or-later, 0BSD: typing-extensions (CPython compound license)
           allow-licenses: >-
             MIT, Apache-2.0, BSD-2-Clause, BSD-3-Clause,
             LGPL-2.0-or-later, LGPL-2.1-only, LGPL-3.0-or-later,
             MPL-1.1, MPL-2.0,
             OFL-1.1,
             PSF-2.0,
             ISC,
-            HPND
+            HPND,
+            Python-2.0,
+            GPL-1.0-or-later,
+            0BSD
+
+          # ✅ Packages whose license cannot be auto-detected
+          # cyclonedx-python-lib is Apache-2.0 (https://github.com/CycloneDX/cyclonedx-python-lib)
+          allow-dependencies-licenses: "pkg:pypi/cyclonedx-python-lib"

.github/workflows/python-tests.yml

@@ -64,6 +64,12 @@ jobs:
         run: |
           uv run python -m compileall infrastructure samples setup shared
 
+      # Supply-chain vulnerability audit (non-blocking)
+      - name: Run pip-audit
+        continue-on-error: true
+        run: |
+          uv run pip-audit
+
       # Run tests and generate coverage reports
       - name: Run pytest with coverage and generate JUnit XML
         id: pytest

SECURITY.md

@@ -1,6 +1,6 @@
 <!-- BEGIN MICROSOFT SECURITY.MD V0.0.9 BLOCK -->
 
-## Security
+# Security
 
 Microsoft takes the security of our software products and services seriously, which includes all source code repositories managed through our GitHub organizations.
 
@@ -16,13 +16,13 @@ You should receive a response within 24 hours. If for some reason you do not, pl
 
 Please include the requested information listed below (as much as you can provide) to help us better understand the nature and scope of the possible issue:
 
-  * Type of issue (e.g. buffer overflow, SQL injection, cross-site scripting, etc.)
-  * Full paths of source file(s) related to the manifestation of the issue
-  * The location of the affected source code (tag/branch/commit or direct URL)
-  * Any special configuration required to reproduce the issue
-  * Step-by-step instructions to reproduce the issue
-  * Proof-of-concept or exploit code (if possible)
-  * Impact of the issue, including how an attacker might exploit the issue
+* Type of issue (e.g. buffer overflow, SQL injection, cross-site scripting, etc.)
+* Full paths of source file(s) related to the manifestation of the issue
+* The location of the affected source code (tag/branch/commit or direct URL)
+* Any special configuration required to reproduce the issue
+* Step-by-step instructions to reproduce the issue
+* Proof-of-concept or exploit code (if possible)
+* Impact of the issue, including how an attacker might exploit the issue
 
 This information will help us triage your report more quickly.
 
@@ -38,6 +38,60 @@ Microsoft follows the principle of [Coordinated Vulnerability Disclosure](https:
 
 <!-- END MICROSOFT SECURITY.MD BLOCK -->
 
+## Repository Security Stance
+
+This repository is a **learning playground** for Azure API Management. Its security posture reflects a deliberate balance: enforce real-world best practices wherever they do not impede the learning workflow, and clearly document every intentional relaxation so users know what to tighten for production.
+
+### What We Enforce
+
+#### Supply chain and CI
+
+* GitHub Actions use least-privilege `permissions:` blocks, SHA-pinned third-party actions, `persist-credentials: false` on `actions/checkout`, and no `pull_request_target`.
+* Dependabot, Dependency Review, and OpenSSF Scorecard are configured.
+* `pip-audit` runs in CI as a non-blocking supply-chain vulnerability check.
+* `uv.lock` provides reproducible builds; `pyproject.toml` pins minimum versions for known-vulnerable packages.
+
+#### Secrets hygiene
+
+* No real secrets are committed. `.gitignore` and `.dockerignore` coverage is comprehensive.
+* Managed identity and APIM named values are used for backend credentials; no hardcoded keys in policies.
+* APIM subscription keys in Bicep outputs are annotated with `#disable-next-line outputs-should-not-contain-secrets` and an inline comment explaining intent. The Bicep linter rule is set to `warning` globally so that any *unintentional* secret output is caught.
+* Python helpers redact sensitive headers (`api-key`, `Authorization`, `Ocp-Apim-Subscription-Key`, `x-api-key`) before logging. The `print_secret()` utility masks secret values in notebook output.
+* The Azure CLI wrapper's secret-redaction regex covers access tokens, refresh tokens, client secrets, subscription keys, connection strings, storage account keys, and shared access signatures.
+
+#### Authentication and authorization
+
+* JWT validation via `validate-jwt` and `validate-azure-ad-token` is demonstrated in the `authX`, `authX-pro`, and `oauth-3rd-party` samples.
+* APIM policy error handlers return generic messages to callers; detailed diagnostics are emitted to Application Insights via `<trace>`.
+
+#### Development tooling
+
+* Local preview servers (`serve_website.py`, `serve_presentation.py`) bind to `127.0.0.1`, not `0.0.0.0`.
+* Jupyter notebook cell outputs are cleared before commit to prevent leaking resource names or subscription IDs.
+
+### Intentional Compromises for Learning
+
+The following defaults prioritise a frictionless learning experience. **Every one of them is parameterised** and can be flipped to the secure setting for production use.
+
+| Default                                              | Why                                                                                                      | Production guidance                                                  |
+| ---------------------------------------------------- | -------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------- |
+| `apimPublicAccess = true`                            | Lets learners call APIM from their machine without VPN or private networking setup.                      | Disable public access; use Private Endpoints or VNet integration.    |
+| `useStrictNsg = false`                               | Avoids NSG rules that block notebook or CLI connectivity during experiments.                             | Enable strict NSGs scoped to required ports and sources.             |
+| WAF in `Detection` mode                              | Prevents the WAF from blocking exploratory requests while learners iterate.                              | Switch to `Prevention` mode once rules are tuned.                    |
+| `enablePurgeProtection = false` on Key Vault         | Allows quick tear-down of lab environments without waiting for the purge-protection retention period.    | Enable purge protection.                                             |
+| `revealBackendApiInfo = true` (X-Backend-URL header) | Helps learners observe routing decisions and backend selection in responses.                             | Remove or disable the header; do not expose internal URLs.           |
+| Subscription-key-only auth on admin APIs             | Keeps sample setup simple. Each admin API includes a `SECURITY` comment pointing to `authX`/`authX-pro`. | Layer JWT validation (`validate-azure-ad-token` or `validate-jwt`).  |
+| APIM subscription keys returned in Bicep outputs     | Notebooks need keys to generate test traffic. Outputs use `#disable-next-line` with an intent comment.   | Fetch keys via RBAC-controlled mechanisms; remove them from outputs. |
+
+### What Is Not a Vulnerability
+
+These items were reviewed and intentionally not flagged:
+
+* **`allowInsecureTls=True` for Application Gateway infras** -- correctly scoped to AppGW infrastructure types (self-signed certificate by design); the flag is `False` for all other infrastructures.
+* **`shell=True` in Azure CLI wrappers** -- commands are constructed from controlled internal strings, never from user-supplied input.
+* **Application Insights instrumentation keys in Bicep outputs** -- Microsoft treats these as non-secret connection identifiers.
+* **`ast.literal_eval` fallback in JSON parsing** -- this is the safe `ast` module function, not `eval`.
+
 ## Security Scanning Scope
 
 This repository is scanned by [OpenSSF Scorecard](https://github.com/ossf/scorecard) via a scheduled GitHub Action. Some checks will report a low score by design; the rationale is recorded as maintainer annotations in [`.github/scorecard.yml`](.github/scorecard.yml) and summarised below.
@@ -48,8 +102,8 @@ This repository does not implement dedicated fuzz testing, and the Scorecard Fuz
 
 Fuzz testing is most valuable where code parses untrusted, attacker-controlled input — file formats, network protocols, deserialisers — particularly in memory-unsafe languages. This repository is a learning playground composed of Bicep templates, Jupyter notebooks, APIM policy XML, and thin Python wrappers around the Azure CLI. None of these components parse untrusted input locally:
 
-- Bicep, policy XML, and notebooks are declarative assets consumed by Azure-side tooling, not by code in this repository.
-- The Python helpers read output from the operator's own `az` CLI session and their own policy files.
-- The only parsing surface (`shared/python/json_utils.py`) delegates to the Python standard library `json` and `ast` modules, which are [already fuzzed upstream in CPython via OSS-Fuzz](https://github.com/google/oss-fuzz/tree/master/projects/cpython3).
+* Bicep, policy XML, and notebooks are declarative assets consumed by Azure-side tooling, not by code in this repository.
+* The Python helpers read output from the operator's own `az` CLI session and their own policy files.
+* The only parsing surface (`shared/python/json_utils.py`) delegates to the Python standard library `json` and `ast` modules, which are [already fuzzed upstream in CPython via OSS-Fuzz](https://github.com/google/oss-fuzz/tree/master/projects/cpython3).
 
 Scorecard additionally has no Python-native fuzzer detection — only OSS-Fuzz enrolment, ClusterFuzzLite, or language-native fuzzers for Go, Haskell, JavaScript/TypeScript, and Erlang are recognised. Adding `hypothesis` or `atheris` property-based tests would therefore not change the score, and would only exercise standard-library code paths already covered upstream.

bicepconfig.json

@@ -10,7 +10,7 @@
             "level": "off"
           },
           "outputs-should-not-contain-secrets": {
-            "level": "off"
+            "level": "warning"
           }
         }
       }

pyproject.toml

@@ -27,6 +27,7 @@ dev = [
   "pytest>=9.0.0",
   "pytest-cov>=7.0.0",
   "coverage>=7.6.4",
+  "pip-audit>=2.7.0",
 ]
 
 [tool.ruff]

samples/azure-maps/map_default_route_v2_aad_get.xml

@@ -43,19 +43,16 @@
                         <cache-store-value key="azure-maps-sas-token" value="@((string)context.Variables["sas-token"])" duration="3600" />
                     </when>
                     <otherwise>
-                        <!-- Log error and return error response -->
-                        <set-variable name="error-message" value="@("Failed to retrieve SAS token. Status: " + ((IResponse)context.Variables["sas-response"]).StatusCode.ToString() + ", Body: " + ((IResponse)context.Variables["sas-response"]).Body.As<string>())" />
+                        <!-- Log detailed error to App Insights; return only a generic message to the caller -->
+                        <trace source="AzureMapsPolicy" severity="error">
+                            @("SAS token request failed. Status: " + ((IResponse)context.Variables["sas-response"]).StatusCode.ToString() + ", Body: " + ((IResponse)context.Variables["sas-response"]).Body.As<string>())
+                        </trace>
                         <return-response>
                             <set-status code="500" reason="Internal Server Error" />
                             <set-header name="Content-Type" exists-action="override">
                                 <value>application/json</value>
                             </set-header>
-                            <set-body>@{
-                                return new JObject(
-                                    new JProperty("error", "SAS token generation failed"),
-                                    new JProperty("details", (string)context.Variables["error-message"])
-                                ).ToString();
-                            }</set-body>
+                            <set-body>{"error": "SAS token generation failed"}</set-body>
                         </return-response>
                     </otherwise>
                 </choose>

samples/costing/main.bicep

@@ -305,6 +305,7 @@ output costExportName string = costExportOutputName
 @description('Subscription keys for the business units')
 output subscriptionKeys array = [for (bu, i) in businessUnits: {
   name: bu.name
+  #disable-next-line outputs-should-not-contain-secrets // Intentional: notebook needs keys for traffic generation
   primaryKey: listSecrets(subscriptions[i].id, '2024-06-01-preview').primaryKey
 }]
 

samples/costing/pf-extract-caller-id.xml

@@ -9,6 +9,12 @@
     back to the APIM subscription ID, or 'unknown' when neither is
     available.
 
+    IMPORTANT: This fragment reads the JWT payload WITHOUT verifying the
+    signature. It assumes a parent policy (e.g. validate-jwt or
+    validate-azure-ad-token) has already validated the token before this
+    fragment executes. Do not use this fragment without upstream JWT
+    validation.
+
     Usage:
         <include-fragment fragment-id="Extract-CallerId" />
         After inclusion, use @((string)context.Variables["callerId"])

samples/dynamic-cors/admin-clear-cache.xml

@@ -18,6 +18,18 @@
         <base />
         <!-- Extract the cache key from the URL path parameter -->
         <set-variable name="cacheKey" value="@(context.Request.MatchedParameters["cacheKey"])" />
+        <!-- Validate cache key format: alphanumeric, hyphens, underscores, max 64 characters -->
+        <choose>
+            <when condition="@(!System.Text.RegularExpressions.Regex.IsMatch((string)context.Variables["cacheKey"], @"^[A-Za-z0-9_\-]{1,64}$"))">
+                <return-response>
+                    <set-status code="400" reason="Bad Request" />
+                    <set-header name="Content-Type" exists-action="override">
+                        <value>application/json</value>
+                    </set-header>
+                    <set-body>{"error": "Invalid cache key. Use 1-64 alphanumeric characters, hyphens, or underscores."}</set-body>
+                </return-response>
+            </when>
+        </choose>
         <!-- Remove the entry from the APIM internal cache (no-op if absent) -->
         <cache-remove-value key="@((string)context.Variables["cacheKey"])" />
         <return-response>

samples/dynamic-cors/admin-load-cache.xml

@@ -34,6 +34,30 @@
         <base />
         <!-- Extract the cache key from the URL path parameter -->
         <set-variable name="cacheKey" value="@(context.Request.MatchedParameters["cacheKey"])" />
+        <!-- Validate cache key format: alphanumeric, hyphens, underscores, max 64 characters -->
+        <choose>
+            <when condition="@(!System.Text.RegularExpressions.Regex.IsMatch((string)context.Variables["cacheKey"], @"^[A-Za-z0-9_\-]{1,64}$"))">
+                <return-response>
+                    <set-status code="400" reason="Bad Request" />
+                    <set-header name="Content-Type" exists-action="override">
+                        <value>application/json</value>
+                    </set-header>
+                    <set-body>{"error": "Invalid cache key. Use 1-64 alphanumeric characters, hyphens, or underscores."}</set-body>
+                </return-response>
+            </when>
+        </choose>
+        <!-- Reject oversized request bodies (max 64 KB) -->
+        <choose>
+            <when condition="@(int.Parse(context.Request.Headers.GetValueOrDefault("Content-Length", "0")) > 65536)">
+                <return-response>
+                    <set-status code="413" reason="Payload Too Large" />
+                    <set-header name="Content-Type" exists-action="override">
+                        <value>application/json</value>
+                    </set-header>
+                    <set-body>{"error": "Request body exceeds the 64 KB size limit."}</set-body>
+                </return-response>
+            </when>
+        </choose>
         <!-- Read and validate the request body -->
         <set-variable name="cacheValue" value="@(context.Request.Body.As<string>(preserveContent: true))" />
         <choose>