Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 80 additions & 2 deletions .github/workflows/publish-verified-model.yml
Original file line number Diff line number Diff line change
@@ -1,8 +1,86 @@
name: Publish Verified Model

on:
workflow_dispatch:
inputs:
model_name:
description: "Name to put in ovllm_manifest.json"
required: true
default: "gpt10m-shakespeare"
hf_repo_id:
description: "Optional Hugging Face repo id, e.g. owner/gpt10m-shakespeare"
required: false
default: ""
publish_to_hf:
description: "Upload the signed directory to Hugging Face"
required: true
default: false
type: boolean

permissions:
contents: read
id-token: write

jobs:
placeholder-job:
sign-and-verify:
runs-on: ubuntu-latest

steps:
- run: echo "This is just a temporary placeholder"
- name: Checkout
uses: actions/checkout@v4
Comment on lines +29 to +30

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win

Set persist-credentials: false on checkout.

zizmor flags credential persistence (artipacked) here — the ambient GITHUB_TOKEN stays in the local git config for the rest of the job unless explicitly disabled.

🔒 Suggested fix
       - name: Checkout
         uses: actions/checkout@v4
+        with:
+          persist-credentials: false
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Checkout
uses: actions/checkout@v4
- name: Checkout
uses: actions/checkout@v4
with:
persist-credentials: false
🧰 Tools
🪛 zizmor (1.26.1)

[warning] 29-30: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-verified-model.yml around lines 29 - 30, The
Checkout step in the publish-verified-model workflow is persisting the ambient
GITHUB_TOKEN in git config, so update the actions/checkout@v4 usage to disable
credential persistence by setting persist-credentials to false on that step.
Locate the Checkout job entry in the workflow and keep the change scoped to that
checkout action invocation.

Source: Linters/SAST tools


- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: "3.12"
cache: "pip"

- name: Install dependencies
run: |
python -m pip install -r requirements.txt
python -m pip install -e .

- name: Train real model (smoke steps)
run: |
python run_experiment.py \
--model gpt10m \
--dataset shakespeare \
--precision fp32 \
--deterministic on \
--steps 8 \
--batch-size 8 \
--block-size 64 \
--keep-artifact

- name: Prepare publish directory
run: |
ovllm prepare-publish \
--weights artifacts/gpt10m_shakespeare_fp32_deton_s99.safetensors \
--out dist/gpt10m-shakespeare \
--name "${{ inputs.model_name }}"
Comment on lines +55 to +60

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Harden run: blocks against script injection from workflow inputs.

inputs.model_name and inputs.hf_repo_id are interpolated directly into run: bash via ${{ }}, which substitutes the raw string into the script before the shell parses it — a value containing shell metacharacters can inject arbitrary commands. Static analysis (zizmor) flags this at lines 61 and 87.

🔒 Suggested fix: pass inputs through env vars
       - name: Prepare publish directory
         run: |
           ovllm prepare-publish \
             --weights openverifiable-smoke.safetensors \
             --out dist/openverifiable-smoke \
-            --name "${{ inputs.model_name }}"
+            --name "$MODEL_NAME"
+        env:
+          MODEL_NAME: ${{ inputs.model_name }}
       - name: Publish to Hugging Face
         if: ${{ inputs.publish_to_hf && inputs.hf_repo_id != '' }}
         env:
           HF_TOKEN: ${{ secrets.HF_TOKEN }}
+          HF_REPO_ID: ${{ inputs.hf_repo_id }}
         run: |
-          ovllm publish-hf "${{ inputs.hf_repo_id }}" dist/openverifiable-smoke
+          ovllm publish-hf "$HF_REPO_ID" dist/openverifiable-smoke

Also applies to: 82-88

🧰 Tools
🪛 zizmor (1.26.1)

[error] 61-61: code injection via template expansion (template-injection): may expand into attacker-controllable code

(template-injection)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/publish-verified-model.yml around lines 56 - 61, Harden
the workflow’s shell steps by removing direct `${{ inputs.model_name }}` and
`${{ inputs.hf_repo_id }}` interpolation from the `run:` blocks. Update the
“Prepare publish directory” and the later publish step to read these values from
environment variables instead, so the shell never parses raw workflow input as
script. Keep the existing `ovllm prepare-publish` and publish logic the same,
but reference the inputs via env in the workflow step definition rather than
inline expansion.

Source: Linters/SAST tools


- name: Sign with GitHub Actions OIDC
env:
SIGSTORE_IDENTITY: "https://github.com/${{ github.repository }}/.github/workflows/publish-verified-model.yml@${{ github.ref }}"
SIGSTORE_IDENTITY_PROVIDER: "https://token.actions.githubusercontent.com"
run: |
ovllm sign dist/gpt10m-shakespeare \
--identity "$SIGSTORE_IDENTITY" \
--identity-provider "$SIGSTORE_IDENTITY_PROVIDER" \
--use-ambient-credentials

- name: Verify signed artifact
run: ovllm verify dist/gpt10m-shakespeare --skip-replay

- name: Upload signed model directory
uses: actions/upload-artifact@v4
with:
name: gpt10m-shakespeare-signed
path: dist/gpt10m-shakespeare

- name: Publish to Hugging Face
if: ${{ inputs.publish_to_hf && inputs.hf_repo_id != '' }}
env:
HF_TOKEN: ${{ secrets.HF_TOKEN }}
run: |
ovllm publish-hf "${{ inputs.hf_repo_id }}" dist/gpt10m-shakespeare
7 changes: 7 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ venv/

__pycache__/
*.pyc
build/
dist/
*.egg-info/
.ovllm-cache/


.vscode/
Expand Down Expand Up @@ -39,3 +43,6 @@ data/*.zip
data/*.tar.gz
data/cifar-10-batches-py/
!data/shakespeare_sample.txt

# Local smoke tests and temp files
openverifiable-smoke/
14 changes: 7 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,12 @@ Dependencies are pinned and hash-locked. This is deliberate: it's both a reprodu
git clone https://github.com/<your-username>/OpenVerifiableLLM
cd OpenVerifiableLLM

# install the exact, locked dependency set
uv sync
# install dependencies and the editable CLI
pip install -r requirements.txt
pip install -e .

# confirm things work
pytest tests/falsifiability
ruff check .
python -m unittest discover -s tests
```

If you need to add a dependency, add it through the lockfile so the hash-locked set stays authoritative. An unpinned dependency undermines both reproducibility and the project's supply-chain posture.
Expand All @@ -58,8 +58,8 @@ If you need to add a dependency, add it through the lockfile so the hash-locked

**Tests.**

- Run the test suite and `ruff` locally before opening a PR. The determinism checks in CI must pass; a PR that breaks them won't merge.
- Code that changes verification behavior, serialization, RNG/optimizer state handling, or the manifest should include tests for both the success case and the failure case. For verification-affecting changes, extend the falsifiability suite so a clean run passes and the relevant tampered run fails.
- Run the test suite (`python -m unittest discover -s tests`) locally before opening a PR. The determinism checks in CI must pass; a PR that breaks them won't merge.
- Code that changes verification behavior, serialization, RNG/optimizer state handling, or the manifest should include tests for both the success case and the failure case. For verification-affecting changes, extend the test suite so a clean run passes and the relevant tampered run fails.
- A change must not silently weaken the bit-exact reproducibility guarantee on the supported single-GPU stack. If a change trades determinism for performance, make that tradeoff explicit and document it.

**Docs.** If a change alters behavior, inputs, the manifest format, or the verification contract, update the relevant docs in the same PR. The manifest schema is a versioned contract that other parts of the project depend on, so changes there need maintainer sign-off and a migration note.
Expand All @@ -68,7 +68,7 @@ If you need to add a dependency, add it through the lockfile so the hash-locked

- Reference the issue it addresses (`Closes #123`) if there is one.
- Describe what changed and why, and flag anything you'd like reviewers to look at closely.
- Confirm the test suite and `ruff` pass.
- Confirm the test suite passes.
- Keep each PR to one logical change. Small, focused PRs get reviewed and merged faster than large, multi-purpose ones.

Maintainers review on a volunteer basis, so a clear, well-tested, well-scoped PR is the best way to get a quick response. Expect some back-and-forth; review comments are about the work, not about you.
Expand Down
Loading