-
Notifications
You must be signed in to change notification settings - Fork 184
fix(ci): stop leaking vaults and add cleanup safety net #910
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
7d06067
9637c69
ba240e4
692db3f
616cf33
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,70 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: cleanup-ci-vaults | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # Manual only. Prefer deleting a single run prefix; optional orphan-defaults for backlog. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| workflow_dispatch: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inputs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| mode: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "prefix = one workflow run; orphan-defaults = drain leaked name=default vaults" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: true | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: "prefix" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: choice | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "prefix" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "orphan-defaults" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| prefix: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "Required for mode=prefix (e.g. ci-32862931541-execution)" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| min_age_hours: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "For orphan-defaults: only delete default vaults older than this many hours" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: "2" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| dry_run: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| description: "List vaults without deleting" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| required: false | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default: "false" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: choice | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| options: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "false" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - "true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cleanup: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| runs-on: blacksmith-4vcpu-ubuntu-2404 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timeout-minutes: 30 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| NOTTE_API_KEY: ${{ secrets.NOTTE_API_KEY }} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| DISABLE_TELEMETRY: "true" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Checkout code | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Install uv | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: astral-sh/setup-uv@v5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Set up Python | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| uses: actions/setup-python@v5 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| python-version-file: ".python-version" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Install notte-sdk | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: uv pip install --system notte-sdk | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. P3: New cleanup workflow installs notte-sdk from PyPI unpinned in a job that exposes the NOTTE_API_KEY secret
Pin notte-sdk to an exact version with --require-hashes, or install from the checked-out repo workspace. AI prompt |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| - name: Cleanup vaults | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ARGS=() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ "${{ inputs.dry_run }}" = "true" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ARGS+=(--dry-run) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ "${{ inputs.mode }}" = "orphan-defaults" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ARGS+=(--orphan-defaults --min-age-hours "${{ inputs.min_age_hours }}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if [ -z "${{ inputs.prefix }}" ]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "prefix is required when mode=prefix" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exit 1 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ARGS+=(--prefix "${{ inputs.prefix }}") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+62
to
+68
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win Do not interpolate free-form inputs into Bash source.
Pass workflow inputs through step-level environment variables. Read the variables in Bash. Proposed fix - name: Cleanup vaults
+ env:
+ INPUT_DRY_RUN: ${{ inputs.dry_run }}
+ INPUT_MODE: ${{ inputs.mode }}
+ INPUT_MIN_AGE_HOURS: ${{ inputs.min_age_hours }}
+ INPUT_PREFIX: ${{ inputs.prefix }}
run: |
ARGS=()
- if [ "${{ inputs.dry_run }}" = "true" ]; then
+ if [ "$INPUT_DRY_RUN" = "true" ]; then
ARGS+=(--dry-run)
fi
- if [ "${{ inputs.mode }}" = "orphan-defaults" ]; then
- ARGS+=(--orphan-defaults --min-age-hours "${{ inputs.min_age_hours }}")
+ if [ "$INPUT_MODE" = "orphan-defaults" ]; then
+ ARGS+=(--orphan-defaults --min-age-hours "$INPUT_MIN_AGE_HOURS")
else
- if [ -z "${{ inputs.prefix }}" ]; then
+ if [ -z "$INPUT_PREFIX" ]; then
echo "prefix is required when mode=prefix"
exit 1
fi
- ARGS+=(--prefix "${{ inputs.prefix }}")
+ ARGS+=(--prefix "$INPUT_PREFIX")
fi📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Check: CodeQL[warning] 35-70: Workflow does not contain permissions 🪛 zizmor (1.29.0)[warning] 34-71: overly broad permissions (excessive-permissions): default permissions used due to no permissions: block (excessive-permissions) [error] 62-62: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) [error] 64-64: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) [error] 68-68: code injection via template expansion (template-injection): may expand into attacker-controllable code (template-injection) 🤖 Prompt for AI AgentsSource: Linters/SAST tools |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| python scripts/cleanup_ci_vaults.py "${ARGS[@]}" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| """Docs snippet test hooks. | ||
|
|
||
| Installs CI vault scoping when ``NOTTE_CI_VAULT_PREFIX`` is set so this workflow | ||
| run names vaults under that prefix. Deletion is done by the workflow | ||
| ``Cleanup vaults created by this run`` step (not here) so pytest-xdist workers | ||
| cannot delete vaults still in use by sibling workers. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import importlib.util | ||
| import os | ||
| import sys | ||
| from pathlib import Path | ||
| from typing import Any | ||
|
|
||
|
|
||
| def _load_ci_vault_scope() -> Any | None: | ||
| if not os.environ.get("NOTTE_CI_VAULT_PREFIX"): | ||
| return None | ||
| path = Path(__file__).resolve().parents[2] / "scripts" / "ci_vault_scope.py" | ||
| spec = importlib.util.spec_from_file_location("ci_vault_scope", path) | ||
| if spec is None or spec.loader is None: | ||
| return None | ||
| module = importlib.util.module_from_spec(spec) | ||
| sys.modules["ci_vault_scope"] = module | ||
| spec.loader.exec_module(module) | ||
| return module | ||
|
|
||
|
|
||
| _SCOPE = _load_ci_vault_scope() | ||
| if _SCOPE is not None: | ||
| _SCOPE.install() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,10 +1,9 @@ | ||
| # @sniptest filename=concept_vault.py | ||
| # @sniptest show=5-9 | ||
| # @sniptest show=4-7 | ||
| from notte_sdk import NotteClient | ||
|
|
||
| client = NotteClient() | ||
| with client.Session() as session: | ||
| vault = client.Vault() | ||
| with client.Session() as session, client.Vault() as vault: | ||
| vault.add_credentials(url="https://github.com", email="...", password="...") | ||
| agent = client.Agent(session=session, vault=vault) | ||
| agent.run(task="Login to GitHub") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Complete the aged-orphan cleanup backstop.
The workflow has no scheduled trigger. The cleanup function only selects exact
defaultnames. Stalepytest-*andtest-*vaults therefore remain active until a user manually intervenes. This can return the account to the active-vault limit and cause CI vault creation to fail with HTTP 429..github/workflows/cleanup-ci-vaults.yml#L3-L5: add a scheduled invocation for--orphan-defaults --min-age-hours 2.scripts/ci_vault_scope.py#L177-L177: include the intended legacy ephemeral-name patternsdefault,pytest-*, andtest-*before applying the age check.📍 Affects 2 files
.github/workflows/cleanup-ci-vaults.yml#L3-L5(this comment)scripts/ci_vault_scope.py#L177-L177🤖 Prompt for AI Agents