security(spec-sync): harden against upstream compromise

.github/workflows/spec-sync.yml

@@ -1,14 +1,33 @@
 name: Weekly Spec Sync
 
+# Security model (see issue #92, audit finding F-O-01):
+#
+# This workflow fetches OpenAPI/AsyncAPI specs from `docs.kalshi.com`, an
+# external source we do not control. A compromise of upstream (MITM,
+# malicious commit to Kalshi's published spec, etc.) must NOT translate
+# into writable code in this repo. Therefore:
+#
+#   - `permissions:` grants only `contents: read` + `issues: write`.
+#     We never push a branch, never open a PR, never touch repo contents.
+#   - Detected drift opens a NEW `spec-drift`-labeled issue, one per
+#     distinct drift fingerprint (sha256 of openapi_sha + asyncapi_sha).
+#     Re-running against the same upstream is a no-op — the existing
+#     open issue dedups. A maintainer locally runs `scripts/sync_spec.py`
+#     + `scripts/generate.py`, audits `kalshi/_generated/models.py`, and
+#     opens a PR with `Closes #N` in the body, which auto-closes the
+#     drift issue on merge.
+#   - Third-party actions are pinned to full commit SHAs so a compromised
+#     mutable tag cannot escalate this workflow's narrow read scope.
+
 on:
   schedule:
     # Mondays at 06:00 UTC
     - cron: "0 6 * * 1"
   workflow_dispatch:
 
 permissions:
-  contents: write
-  pull-requests: write
+  contents: read
+  issues: write
 
 concurrency:
   group: spec-sync
@@ -18,10 +37,6 @@ jobs:
   spec-sync:
     runs-on: ubuntu-latest
     steps:
-      # Third-party actions pinned to SHA (not tag) because this workflow holds
-      # contents:write + pull-requests:write. A compromised mutable tag could
-      # weaponize the auto-PR flow. Other workflows in this repo use bare tags
-      # because their permissions are narrower.
       - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683  # v4.2.2
 
       - name: Install uv
@@ -77,7 +92,7 @@ jobs:
             echo "Both specs unchanged — nothing to do."
           fi
 
-      - name: Extract spec metadata for PR body
+      - name: Extract spec metadata for report
         if: steps.diff.outputs.changed == 'true'
         id: meta
         run: |
@@ -132,85 +147,82 @@ jobs:
           emit_list added_channels "$added_channels"
           emit_list removed_channels "$removed_channels"
 
-      - name: Regenerate models
-        if: steps.diff.outputs.changed == 'true'
-        run: uv run python scripts/generate.py
-
-      - name: Lint
-        if: steps.diff.outputs.changed == 'true'
-        run: uv run ruff check .
-
-      - name: Type check
-        if: steps.diff.outputs.changed == 'true'
-        run: uv run mypy kalshi/
-
-      - name: Run tests
+      - name: Compute spec checksums
         if: steps.diff.outputs.changed == 'true'
-        run: uv run pytest tests/ -v
+        id: checksums
+        run: |
+          set -euo pipefail
+          openapi_sha=$(sha256sum specs/openapi.yaml | awk '{print $1}')
+          asyncapi_sha=$(sha256sum specs/asyncapi.yaml | awk '{print $1}')
+          echo "openapi_sha=${openapi_sha}" >> "$GITHUB_OUTPUT"
+          echo "asyncapi_sha=${asyncapi_sha}" >> "$GITHUB_OUTPUT"
 
-      - name: Compute PR branch date
+      - name: Ensure spec-drift label exists
         if: steps.diff.outputs.changed == 'true'
-        id: date
-        run: echo "today=$(date -u +%Y-%m-%d)" >> "$GITHUB_OUTPUT"
-
-      - name: Create or update spec-sync PR
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          # Idempotent: --force updates if present, creates if not. Removes a
+          # silent-fail footgun where the create step would abort on a
+          # missing label and weekly cron drift would go unreported.
+          gh label create spec-drift \
+            --color 'e4e669' \
+            --description 'Upstream OpenAPI/AsyncAPI spec changed since last sync' \
+            --force \
+            --repo "${GITHUB_REPOSITORY}"
+
+      - name: Open or dedup spec-drift issue
         if: steps.diff.outputs.changed == 'true'
-        uses: peter-evans/create-pull-request@271a8d0340265f705b14b6d32b9829c1cb33d45e  # v7.0.8
-        with:
-          token: ${{ secrets.GITHUB_TOKEN }}
-          branch: chore/spec-sync-${{ steps.date.outputs.today }}
-          delete-branch: true
-          commit-message: "chore: sync Kalshi specs (OpenAPI ${{ steps.meta.outputs.old_version }} -> ${{ steps.meta.outputs.new_version }})"
-          title: "chore: weekly spec sync (OpenAPI ${{ steps.meta.outputs.old_version }} -> ${{ steps.meta.outputs.new_version }})"
-          body: |
-            Automated weekly spec sync from `https://docs.kalshi.com/openapi.yaml`
-            and `https://docs.kalshi.com/asyncapi.yaml`.
-
-            - OpenAPI changed: `${{ steps.diff.outputs.openapi_changed }}`
-            - AsyncAPI changed: `${{ steps.diff.outputs.asyncapi_changed }}`
-
-            ## OpenAPI
-
-            - Version: `${{ steps.meta.outputs.old_version }}` → `${{ steps.meta.outputs.new_version }}`
-            - Endpoints: ${{ steps.meta.outputs.old_path_count }} → ${{ steps.meta.outputs.new_path_count }} paths
-
-            ### Added endpoints
-
-            ```
-            ${{ steps.meta.outputs.added_paths }}
-            ```
-
-            ### Removed endpoints
-
-            ```
-            ${{ steps.meta.outputs.removed_paths }}
-            ```
-
-            ## AsyncAPI
-
-            - Channels: ${{ steps.meta.outputs.old_channel_count }} → ${{ steps.meta.outputs.new_channel_count }}
-
-            ### Added channels
-
-            ```
-            ${{ steps.meta.outputs.added_channels }}
-            ```
-
-            ### Removed channels
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          OPENAPI_CHANGED: ${{ steps.diff.outputs.openapi_changed }}
+          ASYNCAPI_CHANGED: ${{ steps.diff.outputs.asyncapi_changed }}
+          OLD_VERSION: ${{ steps.meta.outputs.old_version }}
+          NEW_VERSION: ${{ steps.meta.outputs.new_version }}
+          OLD_PATH_COUNT: ${{ steps.meta.outputs.old_path_count }}
+          NEW_PATH_COUNT: ${{ steps.meta.outputs.new_path_count }}
+          OLD_CHANNEL_COUNT: ${{ steps.meta.outputs.old_channel_count }}
+          NEW_CHANNEL_COUNT: ${{ steps.meta.outputs.new_channel_count }}
+          ADDED_PATHS: ${{ steps.meta.outputs.added_paths }}
+          REMOVED_PATHS: ${{ steps.meta.outputs.removed_paths }}
+          ADDED_CHANNELS: ${{ steps.meta.outputs.added_channels }}
+          REMOVED_CHANNELS: ${{ steps.meta.outputs.removed_channels }}
+          OPENAPI_SHA: ${{ steps.checksums.outputs.openapi_sha }}
+          ASYNCAPI_SHA: ${{ steps.checksums.outputs.asyncapi_sha }}
+          RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
+        run: |
+          set -euo pipefail
 
-            ```
-            ${{ steps.meta.outputs.removed_channels }}
-            ```
+          # Drift fingerprint: sha256(openapi_sha || asyncapi_sha). Stable across
+          # re-runs against the same upstream; changes the moment either spec does.
+          FINGERPRINT=$(printf '%s%s' "${OPENAPI_SHA}" "${ASYNCAPI_SHA}" | sha256sum | awk '{print $1}')
+          export FINGERPRINT
+
+          # Dedup: skip if an open spec-drift issue already embeds this fingerprint.
+          existing=$(gh issue list \
+            --repo "${GITHUB_REPOSITORY}" \
+            --state open \
+            --label spec-drift \
+            --limit 200 \
+            --json number,body \
+            --jq '[.[] | select(.body | contains("fingerprint:'"${FINGERPRINT}"'"))] | first | .number // empty')
+
+          if [ -n "${existing}" ]; then
+            echo "Drift fingerprint ${FINGERPRINT} already tracked in issue #${existing}; nothing to do."
+            exit 0
+          fi
 
-            ## Changes included
+          # Render body via Python (string.Template.substitute — no shell expansion of
+          # upstream-sourced values). Pipe through --body-file to avoid argv size limits.
+          body_file=$(mktemp)
+          trap 'rm -f "${body_file}"' EXIT
+          python3 scripts/render_drift_body.py > "${body_file}"
 
-            - Updated `specs/openapi.yaml` and/or `specs/asyncapi.yaml`.
-            - Regenerated `kalshi/_generated/models.py` via `scripts/generate.py`.
-            - `uv run ruff check .`, `uv run mypy kalshi/`, and `uv run pytest tests/ -v`
-              all passed on the bot's branch.
+          today=$(date -u '+%Y-%m-%d')
+          title="Spec drift ${today}: openapi ${OLD_VERSION} → ${NEW_VERSION}"
 
-            Review the diff for breaking vs non-breaking changes before merging.
-            Contract tests (`tests/test_contracts.py`) will surface any drift
-            between the spec and the hand-crafted SDK surface.
-          labels: |
-            spec-drift
+          gh issue create \
+            --repo "${GITHUB_REPOSITORY}" \
+            --label spec-drift \
+            --title "${title}" \
+            --body-file "${body_file}"

scripts/render_drift_body.py

@@ -0,0 +1,85 @@
+"""Render the spec-drift issue body from environment variables.
+
+Reads every template placeholder from os.environ and writes the substituted
+markdown to stdout. Uses string.Template.substitute, which performs only
+literal-string substitution — values are never re-interpreted as shell,
+template syntax, or code. This is the safe replacement for the prior
+heredoc-based body construction in spec-sync.yml, which used an unquoted
+heredoc that would expand $(...) command substitutions present in
+upstream-controlled values.
+"""
+
+import os
+import sys
+from string import Template
+
+TEMPLATE = Template("""## Spec drift detected
+
+Upstream specs changed since the last sync. This workflow runs with
+`contents: read` only — it does NOT push a branch or open a PR.
+
+**To resolve:** locally run `uv run python scripts/sync_spec.py` +
+`uv run python scripts/generate.py`, audit the diff (including
+`kalshi/_generated/models.py`), and open a PR whose body contains
+`Closes #<this issue number>` so this issue auto-closes on merge.
+
+- OpenAPI changed: `${OPENAPI_CHANGED}`
+- AsyncAPI changed: `${ASYNCAPI_CHANGED}`
+- Workflow run: ${RUN_URL}
+
+### Spec checksums (sha256 of fetched upstream content)
+
+- `specs/openapi.yaml`: `${OPENAPI_SHA}`
+- `specs/asyncapi.yaml`: `${ASYNCAPI_SHA}`
+
+Reproduce locally and verify the same hashes before generating models.
+
+## OpenAPI
+
+- Version: `${OLD_VERSION}` → `${NEW_VERSION}`
+- Endpoints: ${OLD_PATH_COUNT} → ${NEW_PATH_COUNT} paths
+
+### Added endpoints
+
+```
+${ADDED_PATHS}
+```
+
+### Removed endpoints
+
+```
+${REMOVED_PATHS}
+```
+
+## AsyncAPI
+
+- Channels: ${OLD_CHANNEL_COUNT} → ${NEW_CHANNEL_COUNT}
+
+### Added channels
+
+```
+${ADDED_CHANNELS}
+```
+
+### Removed channels
+
+```
+${REMOVED_CHANNELS}
+```
+
+<!-- spec-sync-bot: do not edit
+fingerprint:${FINGERPRINT}
+-->
+""")
+
+
+def main() -> int:
+    try:
+        sys.stdout.write(TEMPLATE.substitute(os.environ))
+    except KeyError as exc:
+        sys.exit(f"render_drift_body.py: missing environment variable {exc}")
+    return 0
+
+
+if __name__ == "__main__":
+    raise SystemExit(main())