fix(registry): verify registry artifact content hashes#2763
fix(registry): verify registry artifact content hashes#2763daryllimyt wants to merge 11 commits into
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
daryllimyt
added
engine
|
✅ No security or compliance issues detected. Reviewed everything up to 1fe4c3d. Security Overview
Detected Code Changes
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
tracecat/tracecat/executor/registry_artifacts.py
Lines 707 to 711 in 0f8c51f
Enforce hash verification on SquashFS fallback tarball
When a lock carries an expected artifact hash, integrity failures on the primary .squashfs candidate are currently bypassable because the fallback TarballArtifact is created without any expected hash. In materialize(), candidate failures are caught and the next candidate is tried, so a hash mismatch (or other download integrity error) on the squashfs can still lead to executing an unverified tarball sibling if it exists. This weakens the new content-hash verification guarantee for artifact resolution.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
2 issues found across 29 files
Confidence score: 3/5
- There is concrete integrity risk in
tracecat/executor/registry_artifacts.py: in the TAR_GZ flow,expected_hashis not applied toTarballArtifact, andTarballArtifact.downloaddoes not enforce hash verification, so tampered artifacts could be accepted. tracecat/registry/versions/schemas.pycurrently accepts anyartifact_hashstring despite SHA-256 documentation, which can let invalid data in and weaken downstream validation guarantees.- Given the medium-high severity and confidence on artifact verification behavior, this looks mergeable only with caution rather than low-risk safe-to-merge.
- Pay close attention to
tracecat/executor/registry_artifacts.py,tracecat/registry/versions/schemas.py- missing hash enforcement and weak input validation are the main regression/integrity concerns.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc4c4c0d4b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 181ebe0a7a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
tracecat/tracecat/registry/sync/jobs.py
Line 234 in 7f01567
Persist artifact hash after fresh-install deferred build
When startup sync runs on a fresh install, this call path sets promote_version_id=None, so _build_platform_registry_artifact() never invokes _promote_platform_registry_version_after_artifact_build() to write back the built artifact metadata. In the same flow, deferred sync can create the version with artifact_hash=None when no prebuilt metadata is present (tracecat/registry/sync/base_service.py lines 479-487). The result is a current builtin registry version that keeps a null hash even after the artifact is built, so lock resolution won’t emit hash-locked artifact refs and executors skip integrity verification indefinitely in that environment.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
daryllimyt
force-pushed
the
build/registry-artifact-content-hash
branch
from
7f01567 to
56416c6
Compare
daryllimyt
force-pushed
the
build/registry-artifact-content-hash
branch
from
56416c6 to
07618f2
Compare
daryllimyt
force-pushed
the
graphite-base/2763
branch
from
2f03d6d to
0023fcb
Compare
daryllimyt
changed the base branch from
main
to
fix/registry-current-startup-artifact-uri
2 participants
Summary
#sha256=...artifact references so executors verify downloaded SquashFS artifactsgzip -6Stack
Stacked on #2753, which keeps the prebuilt manifest startup-sync performance changes and manifest-fingerprint fallback.
Testing
uv run pytest tests/unit/executor/test_registry_helpers.py tests/unit/test_registry_artifacts.py tests/unit/test_registry_sync_base_service.py tests/unit/test_registry_lock_service.py tests/unit/test_registry_platform_startup.py tests/unit/test_registry_sync_runner.py tests/unit/test_registry_sync_schemas.pyuv run pytest tests/unit/test_registry_sync_artifact.pyuv run ruff check .uv run ruff format --check .uv run pyright tracecat/config.py tracecat/registry/sync/artifact.py tests/unit/test_registry_sync_artifact.pyuv run alembic headspnpm -C frontend exec biome check src/client/schemas.gen.ts src/client/types.gen.ts