Conversation
There was a problem hiding this comment.
Pull request overview
Adds SEP-58 reproducible contract builds and source archive generation.
Changes:
- Adds
--verifiablebuild flags and provenance metadata. - Adds deterministic source archiving and
contract archive. - Extends container execution, artifact handling, tests, and documentation.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
FULL_HELP_DOCS.md |
Documents new commands and flags. |
cmd/soroban-cli/src/config/locator.rs |
Adds recursive permission hardening. |
cmd/soroban-cli/src/config/data.rs |
Adds managed archive storage. |
cmd/soroban-cli/src/commands/mod.rs |
Adds verifiable help heading. |
cmd/soroban-cli/src/commands/contract/mod.rs |
Registers archive command. |
cmd/soroban-cli/src/commands/contract/build/verifiable.rs |
Implements verifiable builds. |
cmd/soroban-cli/src/commands/contract/build/source_archive.rs |
Implements reproducible archives. |
cmd/soroban-cli/src/commands/contract/build/container.rs |
Shares container and artifact logic. |
cmd/soroban-cli/src/commands/contract/build.rs |
Adds flags and dispatch. |
cmd/soroban-cli/src/commands/contract/archive.rs |
Implements archive CLI. |
cmd/soroban-cli/src/commands/container/shared.rs |
Adds streamed image pulling. |
cmd/soroban-cli/Cargo.toml |
Adds archive dependencies. |
cmd/crates/soroban-test/tests/it/build.rs |
Adds integration coverage. |
Cargo.lock |
Locks dependency updates. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d58a8a7 to
fce6107
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Suppressed comments (6)
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- Metadata keys are accepted as arbitrary strings by
parse_meta_arg, but this records the key unescaped. A key containing whitespace makes the stampedbldoptsplit into multiple shell words, and shell metacharacters such as;can execute commands when a verifier replays the joined options. Escape the key portion as well as the value (or reject non-shell-safe metadata keys).
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
cmd/soroban-cli/src/commands/contract/build/container.rs:760
- The container is explicitly forced to write to
/source/targetviaCARGO_TARGET_DIR, so using the host metadata target here breaks collection whenever the host hasCARGO_TARGET_DIRor a configuredtarget-dir. Plain builds then return a missing/stale path; for verifiable builds an absolute metadata path also causesjointo discard the extracted root and can select an old host WASM instead of the newly built artifact. Collect from the forcedtargetdirectory.
let host_target = md.target_directory.as_std_path();
cmd/soroban-cli/src/commands/contract/build/source_archive.rs:207
- This filter silently omits every symlink, including Git-tracked symlinked files and directories. Such links are part of the source tree and may be required by path dependencies or build scripts, so the archived source can fail to build or differ from the committed source. Preserve safe symlink entries deterministically, or reject them explicitly instead of dropping them.
if entry.file_type().is_some_and(|t| t.is_file()) {
files.push(entry.path().to_path_buf());
}
cmd/soroban-cli/src/commands/contract/build/verifiable.rs:309
- Hardening the extracted tree through this helper changes every file to mode
0600, removing executable bits from Git-tracked helper scripts. A contract whose build script invokes an executable from the repository will build normally but fail only in verifiable mode. Use source-specific hardening that preserves the owner execute bit while removing group/other access; keep config files at0600.
enforce_hardened_tree(tmp.path()).map_err(source_archive::Error::ArchiveExtract)?;
cmd/soroban-cli/src/commands/contract/build/source_archive.rs:129
- Every nonzero
git statusresult is treated as “not a repository.” Failures in a real repository (for example corrupt metadata, ownership checks, or configuration errors) therefore bypass the clean-tree requirement and allow an unverified working tree to be archived. First determine whether this is a work tree, and propagate status failures for repositories; only the explicit non-repository case should proceed.
// Not a git repo (or git refused): can't verify cleanliness, proceed.
if !status.status.success() {
return Ok(false);
}
cmd/soroban-cli/src/commands/contract/build/verifiable.rs:147
- The documented contract says
--verifiableimplies--locked, but this branch knowingly performs an unlocked build. That can update dependency resolution relative to the archived lockfile, so the stamped source/image inputs no longer guarantee the advertised reproducibility. Reject images older than the--lockedminimum for verifiable builds instead of degrading to an unlocked build.
} else {
print.warnln(
"The build image's `contract build` does not support --locked; \
building without it. Dependency drift may affect reproducibility.",
);
}
fce6107 to
4988080
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Suppressed comments (2)
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- Only escaping
vdoes not make every recorded option valid shell syntax becauseparse_meta_argallows metadata keys containing spaces or shell metacharacters. For example,--meta 'my key=value'is forwarded as one argv item but recorded as--meta=my key=value, which splits into two arguments during replay. Escape the key segment as well so the recorded bldopt round-trips.
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
cmd/soroban-cli/src/commands/contract/build/verifiable.rs:149
- A verifiable build is documented to imply
--locked, but an older pinned image reaches this branch and the build continues without it. That permits dependency resolution to drift between the original build and a verifier's replay, defeating the reproducibility guarantee. Please reject images whose CLI does not support--lockedinstead of producing a “verifiable” artifact without it.
} else {
print.warnln(
"The build image's `contract build` does not support --locked; \
building without it. Dependency drift may affect reproducibility.",
);
4988080 to
f1ab06b
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 3 comments.
Suppressed comments (1)
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- SEP-58 defines each
bldoptas one value passed verbatim as an argv argument (“as if single-quoted”), not as shell source to evaluate. Escaping onlyvstores literal quote characters: an original--meta=note=added on buildis recorded as--meta=note='added on build', so a conforming verifier passes the apostrophes into the metadata value and cannot reproduce the WASM. Record the raw{key}={v}argument instead, and update the shell-roundtrip test/documentation accordingly.
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
f1ab06b to
42db3e5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 14 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
cmd/soroban-cli/src/commands/contract/build/source_archive.rs:127
git status --porcelainhides files ignored by global excludes,.git/info/exclude, and parent ignore files. The archive walker explicitly disables those sources, so a machine-local file such as a globally ignored.envcan pass this “clean tree” gate and then be included and persisted in the archive. Validate cleanliness against the actual selected archive entries (for example, reject selected files that are not tracked) so local ignored files cannot leak or makesource_sha256machine-specific.
.arg("status")
.arg("--porcelain")
42db3e5 to
434b938
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Recorded build options and failure reproduction commands currently cannot reliably reproduce the original build.
Get a fresh assessment by requesting another Copilot review.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 2
- Review effort level: Balanced
| if let Some(v) = value { | ||
| args.push(format!("{key}={v}")); | ||
| if record_bldopts { | ||
| bldopts.push(format!("{key}={}", shell_escape::escape(v.into()))); |
| container::run_in_container( | ||
| &image_ref, | ||
| &resolved.mount_root, | ||
| &container_cmds, | ||
| &env, |
### What Adds a `stellar contract build archive` subcommand that generates — or inspects — a byte-reproducible source archive of the working tree. Split out of #2709 so it can be reviewed on its own; it's the shared foundation the `--verifiable` build is stacked on. The archive walks the working directory honoring the project's own `.gitignore`/`.ignore` files (`.git` is always skipped), so the same tree hashes to the same `source_sha256`. It refuses a working tree it can't verify against committed source (dirty, or files marked assume-unchanged/skip-worktree), is written `0600`, and the `--dry-run` listing is sanitized against hostile filenames. ### Why SEP-58 verifies that a deployed WASM came from a specific source; this archive is the exact artifact its `source_sha256` refers to. ### Known limitations N/A
434b938 to
23d357e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Workspace-member builds fail and shell-escaped bldopt metadata cannot be replayed verbatim as SEP-58 requires.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (1)
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- SEP-58 replays each
bldoptverbatim as one argv argument, not as shell source. Escapingvstores literal quote characters (for example--features='a b'), so a verifier passes a different value and cannot reproduce the build. Record the raw{key}={v}argument and update the shell-oriented test accordingly.
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
- Files reviewed: 9/9 changed files
- Comments generated: 3
- Review effort level: Balanced
| // The source root is the current working directory: it's archived, | ||
| // bind-mounted into the container, and the `--manifest-path` bldopt is | ||
| // relativized against it. Run from the project/workspace root you want built. | ||
| let source_root = source_archive::resolve_source_root(); |
| #[arg( | ||
| long, | ||
| requires = "verifiable", | ||
| requires = "source_sha256", | ||
| help_heading = HEADING_VERIFIABLE | ||
| )] |
| /// party can reproduce the exact bytes. Implies `--locked`. Requires a clean | ||
| /// git tree. Requires `--image` pinned by digest |
23d357e to
f6b8201
Compare
f6b8201 to
b87725e
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Recorded build options currently violate SEP-58 replay semantics, and valid source-URI usage is rejected.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- SEP-58 defines each
bldoptas one argument replayed verbatim, not as shell source. Escaping only the value inserts literal quotes into metadata—for example,--features=a bbecomes--features='a b', so a conforming verifier passes a different feature value and cannot reproduce the build. Record the original argv string unchanged.
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
cmd/soroban-cli/src/commands/contract/build.rs:160
- This makes
--source-uriunusable unless the caller also supplies the optional hash pin, even though verifiable mode always computes and recordssource_sha256itself. It unnecessarily forces users to pre-generate the archive before they can provide its URI and contradicts the documented standalone--source-urioption; require only--verifiable.
requires = "source_sha256",
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Balanced
| print.infoln(format!("Using Rust toolchain {}", probe.toolchain)); | ||
| env.push(format!("RUSTUP_TOOLCHAIN={}", probe.toolchain)); | ||
|
|
||
| container::run_in_container( |
b87725e to
f6b8201
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Verbatim metadata replay, workspace-member builds, Git enforcement, and source URI handling currently have blocking correctness issues.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- SEP-58 defines each
bldoptas one verbatim argv argument, not a shell fragment. Escaping the value changes the recorded argument—for example, the original--meta=note=added on buildis recorded as--meta=note='added on build', so a verifier that correctly passes it verbatim includes literal quote characters and cannot reproduce the WASM. Record the forwarded argument unchanged.
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
cmd/soroban-cli/src/commands/contract/build.rs:162
source_sha256is always computed and stamped by verifiable mode, so requiring the user to also pass the optional hash pin prevents the valid--verifiable --source-uri ...workflow. This contradicts the flag documentation and SEP-58's optional-URI model; remove this extra requirement.
#[arg(
long,
requires = "verifiable",
requires = "source_sha256",
help_heading = HEADING_VERIFIABLE
)]
cmd/soroban-cli/src/commands/contract/build/verifiable.rs:94
- Using the current directory as the archive/mount root breaks the existing supported workflow of running
contract buildfrom a workspace member. Host metadata still discovers the workspace, but the extracted source omits the rootCargo.lockand sibling members; because verifiable mode forwards--locked, the member-only copy cannot build. Resolve metadata before archiving and use itsworkspace_rootas the source root, while preserving the selected member via--package/--manifest-path.
// The source root is the current working directory: it's archived,
// bind-mounted into the container, and the `--manifest-path` bldopt is
// relativized against it. Run from the project/workspace root you want built.
let source_root = source_archive::resolve_source_root();
cmd/soroban-cli/src/commands/contract/build/verifiable.rs:199
- No integration test exercises a successful verifiable build: every added case exits during validation or fails at the container engine, so archive mounting, probing, metadata stamping, multi-package invocation, and artifact copy-back remain untested. Add an end-to-end test using the repository's fake container-executable pattern (
cmd/crates/soroban-test/tests/it/container.rs:1-7) that completes the build path and inspects the resulting invocation/WASM.
container::run_in_container(
&image_ref,
&resolved.mount_root,
&container_cmds,
&env,
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
| // The archive is the working tree, so refuse a dirty repo: a verifiable build | ||
| // should be deliberate, off a committed state, not whatever happens to be on | ||
| // disk. Skipped when the source root isn't a git repo. | ||
| source_archive::ensure_clean_tree(&source_root, None).map_err(Error::from)?; |
There was a problem hiding this comment.
🟡 Changes recommended
Workspace-root handling and shell-escaped bldopt values currently break valid builds and SEP-58 replay.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
cmd/soroban-cli/src/commands/contract/build/verifiable.rs:94
contract buildis documented to work from a workspace-member subdirectory, but this archives that member directory whilecargo metadatabelow still resolves the enclosing workspace. Fromcontracts/add,/sourcetherefore lacks the workspaceCargo.toml,Cargo.lock, and sibling packages even though workspace packages are inferred and forwarded, so the container build fails or cannot honor--locked. Resolve metadata before archiving and use itsworkspace_root, or reject non-root invocations before creating the archive.
let source_root = source_archive::resolve_source_root();
cmd/soroban-cli/src/commands/contract/build/container.rs:304
- SEP-58 defines each
bldoptvalue as one argument replayed verbatim (as if the whole value were single-quoted), so embedding shell quoting changes the argument. For example, a build receives one--features=foo barargument, but this records--features='foo bar'; a verifier replaying that value as argv passes literal quote characters and gets different or invalid features. Record the raw value instead.
bldopts.push(format!("{key}={}", shell_escape::escape(v.into())));
cmd/soroban-cli/src/commands/contract/build.rs:160
source_sha256is always computed from the archive and stamped intoSourceIds, so requiring the user to also provide the optional hash pin prevents the documented--source-uri-only workflow for no functional reason.--verifiable --source-uri ...is rejected by clap even though the computed hash already satisfies SEP-58; remove this second requirement.
requires = "source_sha256",
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced
| let supports_locked = at_least(container::LOCKED_MIN); | ||
| let supports_optimize_flag = at_least(container::OPTIMIZE_FLAG_MIN); | ||
| let supports_optimize_false = at_least(container::OPTIMIZE_NEW_SYNTAX_MIN); |
| // source_sha256) before the docker stage, so the "Wrote source archive" line | ||
| // appears even though the build then fails to reach a real image. | ||
| #[test] | ||
| fn verifiable_always_writes_source_archive() { |
What
Adds a
--verifiableflag tostellar contract buildthat performs a reproducible build inside a digest-pinned Docker container and stamps SEP-58 metadata (bldimg,source_uri,source_sha256,bldopt) into the resulting WASM so third parties can re-run the build and verify the output byte-for-byte. The container connection/resource flags are the existing container-build arguments, reused here.A
--verifiablebuild always generates the reproducible source archive (the same generator exposed asstellar contract build archive), records its SHA-256 assource_sha256, writes a content-addressed copy to the data dir'sarchives/<sha256>.tar.gz, and builds from the extracted (permission-hardened) copy so the WASM comes from exactly the bytes that were hashed. Each contract is built with its own--package, forwarded to the build and recorded as abldopt, so every WASM is independently reproducible; multi-contract workspaces build in a single container to share the crates download andtarget/. Everybldoptis recorded as valid shell syntax so a verifier can replay the exact invocation.Why
SEP-58 defines how to verify that a deployed contract WASM came from a specific source built with a specific toolchain image. Until now the CLI had no built-in way to produce such a build — users had to assemble the docker invocation, run cargo inside it, and stamp the custom sections by hand. This makes it a first-class option on
stellar contract build, building on the reproducible source archive fromstellar contract build archive.