Rework SEP-58 bldopt into ordered bldarg list#1965
Conversation
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 4679db5625
ℹ️ 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".
| full argument list is recorded, including the subcommand (e.g. `contract`, | ||
| `build`) and every flag. This lets the vocabulary cover any argument the build |
There was a problem hiding this comment.
With bldarg now defined as the full argument list passed to the image entry point, builds that embed SEP-58 data through --meta also pass those --meta bldimg=..., --meta bldarg=..., and source metadata arguments to the same entry point. That is not faithfully representable without either omitting part of the command, as Appendix A does, or recursively recording bldarg metadata, so producers and verifiers can construct different replay commands and fail byte-identical reproduction. Please specify that bldarg excludes the metadata-injection arguments, or define another non-recursive replay rule.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
This PR updates SEP-58 (Contract Build Reproducibility for Verification) to rename the build metadata field from bldopt to bldarg and redefine it as an ordered, verbatim-replayed argument list for reproducible contract builds, expanding support to include positional arguments (e.g. contract, build) and adding guidance on treating producer-supplied arguments as untrusted.
Changes:
- Renames
bldopt→bldargand redefines it as an ordered list of CLI arguments to replay verbatim. - Updates rationale, security concerns, and Appendix A examples to reflect the new field semantics.
- Bumps SEP version to
0.6.0and records the change in the changelog.
| `bldarg` entries are the arguments passed to the build image's default entry | ||
| point, recorded one argument per entry and replayed in the order recorded. The | ||
| full argument list is recorded, including the subcommand (e.g. `contract`, | ||
| `build`) and every flag. This lets the vocabulary cover any argument the build | ||
| command supports without the SEP enumerating them, so new arguments require no | ||
| spec revision. |
| | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------- | | ||
| | `bldimg` | Fully-qualified container image used for the build, pinned by digest. The image bundles the rust toolchain and any other build-time dependencies, so pinning the image transitively pins the toolchain. The digest MUST reference a single-architecture manifest, not a multi-arch manifest list (a.k.a. fat manifest); a multi-arch digest resolves to different concrete images depending on the puller's host architecture, which defeats the pin. The reference MUST include an explicit registry host (with optional port), e.g. `docker.io/repo/image@sha256:...` or `registry.example.com:5000/repo/image@sha256:...`; implicit Docker-Hub-style short references like `repo/image@sha256:...` are not conformant. An optional tag MAY appear before the digest (e.g. `registry/repo/image:tag@sha256:...`) but is informational; the digest alone determines what is pulled. | `^(?:localhost(?::\d+)?|[^\s@/]*[.:][^\s@/]*)/[^\s@]+@sha256:[0-9a-f]{64}$` | | ||
| | `bldopt` | A single shell-style flag passed verbatim as one argument to the build command, as if single-quoted in a shell (e.g. `--profile=release`, `--features=foo,bar`, `--no-default-features`). For flags that take a value, the `=` form keeps the flag and its value in one argument. The entry MAY appear multiple times, once per flag. The order of entries is not significant. | `^--[A-Za-z][A-Za-z0-9_-]*(=.+)?$` | | ||
| | `bldarg` | A single shell-style argument passed verbatim to the build command, as if single-quoted in a shell (e.g. `contract`, `build`, `--profile=release`, `--features=foo,bar`, `--no-default-features`). An argument may be a flag; for flags that take a value, the `=` form keeps the flag and its value in one argument. The entry MAY appear multiple times, once per argument. The order of entries is significant: verifiers MUST replay them in the order recorded. | `^(--[A-Za-z][A-Za-z0-9_-]*(=.+)?|[A-Za-z][A-Za-z0-9_-]*)$` | |
| The `bldarg` entries tune the cargo invocation inside; the container itself is | ||
| what makes the rebuild possible at all. |
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 264f8152bb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| | -------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -------------------------------------------------------------------------------- | | ||
| | `bldimg` | Fully-qualified container image used for the build, pinned by digest. The image bundles the rust toolchain and any other build-time dependencies, so pinning the image transitively pins the toolchain. The digest MUST reference a single-architecture manifest, not a multi-arch manifest list (a.k.a. fat manifest); a multi-arch digest resolves to different concrete images depending on the puller's host architecture, which defeats the pin. The reference MUST include an explicit registry host (with optional port), e.g. `docker.io/repo/image@sha256:...` or `registry.example.com:5000/repo/image@sha256:...`; implicit Docker-Hub-style short references like `repo/image@sha256:...` are not conformant. An optional tag MAY appear before the digest (e.g. `registry/repo/image:tag@sha256:...`) but is informational; the digest alone determines what is pulled. | `^(?:localhost(?::\d+)?|[^\s@/]*[.:][^\s@/]*)/[^\s@]+@sha256:[0-9a-f]{64}$` | | ||
| | `bldopt` | A single shell-style flag passed verbatim as one argument to the build command, as if single-quoted in a shell (e.g. `--profile=release`, `--features=foo,bar`, `--no-default-features`). For flags that take a value, the `=` form keeps the flag and its value in one argument. The entry MAY appear multiple times, once per flag. The order of entries is not significant. | `^--[A-Za-z][A-Za-z0-9_-]*(=.+)?$` | | ||
| | `bldopt` | A single shell-style argument passed verbatim to the build command, as if single-quoted in a shell (e.g. `contract`, `build`, `--profile=release`, `--features=foo,bar`, `--no-default-features`). An argument may be a flag; for flags that take a value, the `=` form keeps the flag and its value in one argument. The entry MAY appear multiple times, once per argument. The order of entries is significant: verifiers MUST replay them in the order recorded. | `^(--[A-Za-z][A-Za-z0-9_-]*(=.+)?|[A-Za-z][A-Za-z0-9_-]*)$` | |
There was a problem hiding this comment.
This now defines each bldopt entry as one verbatim argv item, but the positional side of the regex only accepts bare words. The Stellar CLI reference documents build options such as --manifest-path <MANIFEST_PATH> and --features <FEATURES>, so a producer that actually ran stellar contract build --manifest-path contracts/foo/Cargo.toml would need to record contracts/foo/Cargo.toml as its own argument, which this regex rejects because of the slash and dot. That makes documented invocations non-conformant unless producers rewrite their original argv into --flag=value, contradicting the new verbatim replay rule.
Useful? React with 👍 / 👎.
This reverts commit 264f815.
What
Rename SEP-58's
bldoptbuild-option field tobldargand redefine it as the ordered list of arguments passed to the build image's default entry point, where order is significant and entries are replayed verbatim; broaden the value format to accept positional arguments such ascontractandbuildalongside flags, drop the always-include---lockedreproduction recommendation, and add an argument-trust security concern that lets verifiers accept only a vetted subset of build arguments.Why
The original
bldoptrecorded only the flags that deviated from an assumed baselinestellar contract buildinvocation, which left the actual subcommand implicit and treated argument order as insignificant even though build tooling can be order-sensitive; recording the full ordered argument list makes a recorded build unambiguous to replay. Because those arguments are producer-supplied and replayed against the stellar-cli, the specification now also warns that verifiers must treat them as untrusted — a crafted argument could drive the cli to execute arbitrary commands or reach resources outside the source — and may restrict them to an allowlisted subset, mirroring how they already restrict build images.Discussion about this change is happening here:
Generated by Claude Code