feat(opensmith): add corpus lock and parity harness scaffold - #4
feat(opensmith): add corpus lock and parity harness scaffold#4ludoplex wants to merge 2 commits into
Conversation
Reviewer's GuideAdds a deterministic OpenSmith corpus lock/extraction toolchain and a parity harness scaffold, wired into the Makefile and documented for the PR1 reverse-engineering workflow against Generator-85.zip. Sequence diagram for OpenSmith corpus lock, extract, and parity harnesssequenceDiagram
actor Developer
participant Makefile
participant opensmith_corpus_py as opensmith_corpus_py
participant opensmith_parity_py as opensmith_parity_py
participant Generator_85_zip as Generator_85_zip
participant Corpus_lock_json as corpus_lock_json
participant Corpus_dir as build_opensmith_corpus
participant Parity_dir as build_opensmith_parity
participant Engine as Engine_command
rect rgb(230,230,255)
Developer->>Makefile: make opensmith-corpus-lock OPENSMITH_ZIP=...
Makefile->>opensmith_corpus_py: python opensmith_corpus.py inventory --zip OPENSMITH_ZIP --lock OPENSMITH_LOCK
opensmith_corpus_py->>Generator_85_zip: open and scan outer zip
opensmith_corpus_py->>Generator_85_zip: scan nested zips under Samples prefix
opensmith_corpus_py-->>Makefile: deterministic inventory entries
opensmith_corpus_py->>Corpus_lock_json: write lock (source_zip_sha256, entries, counts_by_ext)
Makefile-->>Developer: corpus.lock.json created
end
rect rgb(230,255,230)
Developer->>Makefile: make opensmith-corpus OPENSMITH_ZIP=...
Makefile->>opensmith_corpus_py: python opensmith_corpus.py extract --zip OPENSMITH_ZIP --lock OPENSMITH_LOCK --out-dir OPENSMITH_CORPUS_DIR
opensmith_corpus_py->>Corpus_lock_json: read lock
opensmith_corpus_py->>Generator_85_zip: compute sha256 and compare to lock
opensmith_corpus_py->>Generator_85_zip: read outer and nested zip members per lock entries
opensmith_corpus_py->>Corpus_dir: write extracted files in outer and nested subtrees
opensmith_corpus_py-->>Makefile: report extracted entries count
Makefile-->>Developer: deterministic corpus extracted
end
rect rgb(255,240,230)
alt ENGINE not set (dry-run)
Developer->>Makefile: make opensmith-parity
Makefile->>opensmith_parity_py: python opensmith_parity.py --inventory OPENSMITH_LOCK --corpus-dir OPENSMITH_CORPUS_DIR --artifacts-dir OPENSMITH_PARITY_DIR --dry-run
opensmith_parity_py->>Corpus_lock_json: read lock
opensmith_parity_py->>Corpus_dir: verify all selected fixture files exist
opensmith_parity_py-->>Developer: dry-run ok: N fixtures discovered
else ENGINE set
Developer->>Makefile: make opensmith-parity ENGINE="engine {input}"
Makefile->>opensmith_parity_py: python opensmith_parity.py --inventory OPENSMITH_LOCK --corpus-dir OPENSMITH_CORPUS_DIR --artifacts-dir OPENSMITH_PARITY_DIR --engine ENGINE
opensmith_parity_py->>Corpus_lock_json: read lock
opensmith_parity_py->>Corpus_dir: collect fixture paths
loop for each fixture
opensmith_parity_py->>Engine: run engine_command with {input}
Engine-->>opensmith_parity_py: stdout, stderr, exitcode
opensmith_parity_py->>Parity_dir: write .stdout, .stderr, .exitcode files
opt baseline configured
opensmith_parity_py->>Parity_dir: compare or write baseline snapshots
end
end
opensmith_parity_py-->>Developer: summary (fixtures run, exit failures, baseline mismatches)
end
end
ER diagram for OpenSmith corpus.lock.json schemaerDiagram
CorpusLock {
int schema
string source_zip
string source_zip_sha256
string nested_zip_prefix
string extensions_json
int total_entries
string counts_by_ext_json
}
CorpusEntry {
string container
string entry
string ext
int size
string sha256
}
CorpusLock ||--o{ CorpusEntry : entries
Flow diagram for opensmith_corpus inventory and extract operationsflowchart TD
A[Start inventory command
opensmith_corpus.py inventory] --> B[Resolve zip path
resolve_existing_path]
B --> C[Open Generator-85.zip as outer zip]
C --> D[Iterate outer zip entries
sorted by filename]
D --> E{Entry is file
and has allowed extension?}
E -->|No| F[Check if entry is nested zip
under nested_zip_prefix]
E -->|Yes| G[Read file bytes
compute sha256
append outer entry]
F --> H{Is nested zip
and under prefix?}
H -->|No| I[Skip entry]
H -->|Yes| J[Open nested zip
from in-memory bytes]
J --> K[Iterate nested entries
sorted by filename]
K --> L{Nested entry has allowed extension?}
L -->|No| M[Skip nested entry]
L -->|Yes| N[Read nested bytes
compute sha256
append nested entry]
G --> O[After all entries
sort by container and entry]
N --> O
O --> P[Compute counts_by_ext
and total_entries]
P --> Q[Compute source_zip_sha256
from file]
Q --> R[Build lock object
schema, source_zip,
nested_zip_prefix,
extensions, entries]
R --> S[Write JSON lock
specs/testing/opensmith/corpus.lock.json]
S --> T[End inventory]
subgraph Extract_corpus
A2[Start extract command
opensmith_corpus.py extract] --> B2[Resolve zip path
and lock path]
B2 --> C2[Read lock JSON
into memory]
C2 --> D2[Optionally verify
zip sha256 matches lock]
D2 --> E2[Create out_dir
build/opensmith/corpus]
E2 --> F2[Open outer zip
and init nested cache]
F2 --> G2[Iterate entries
from lock]
G2 --> H2{Entry container
is outer?}
H2 -->|Yes| I2[Read member
from outer zip]
H2 -->|No| J2[Load or reuse
nested zip from outer]
J2 --> K2[Read member
from nested zip]
I2 --> L2[Compute sha256
and compare to lock]
K2 --> L2
L2 --> M2{Digest matches?}
M2 -->|No| N2[Raise error
sha mismatch]
M2 -->|Yes| O2[Construct safe
relative path under
outer or nested/]
O2 --> P2[Write file bytes
to out_dir preserving
directory structure]
P2 --> Q2{More entries?}
Q2 -->|Yes| G2
Q2 -->|No| R2[Close nested zips
and finish]
R2 --> S2[End extract]
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 security issues, 2 other issues, and left some high level feedback:
Security issues:
- Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'. (link)
- Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'. (link)
- Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead. (link)
General comments:
- In
opensmith_parity.py,--write-baselineis a no-op when--baseline-diris omitted (sincebaseline_dirremainsNone); consider validating that--baseline-diris provided whenever--write-baselineis set and failing fast with a clear error message. - The parity harness currently uses
subprocess.run(..., shell=True)with the engine command; if you expect engines with user-controlled input or want more predictable behavior across platforms, consider parsing the engine into an argv list and avoidingshell=Truewhere possible.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `opensmith_parity.py`, `--write-baseline` is a no-op when `--baseline-dir` is omitted (since `baseline_dir` remains `None`); consider validating that `--baseline-dir` is provided whenever `--write-baseline` is set and failing fast with a clear error message.
- The parity harness currently uses `subprocess.run(..., shell=True)` with the engine command; if you expect engines with user-controlled input or want more predictable behavior across platforms, consider parsing the engine into an argv list and avoiding `shell=True` where possible.
## Individual Comments
### Comment 1
<location path="scripts/opensmith_parity.py" line_range="101-102" />
<code_context>
+ parser.add_argument("--corpus-dir", required=True, help="path to extracted corpus")
+ parser.add_argument("--artifacts-dir", required=True, help="path for run outputs")
+ parser.add_argument("--engine", default="", help="engine command, optional {input} token")
+ parser.add_argument("--baseline-dir", default="", help="golden baseline output directory")
+ parser.add_argument("--write-baseline", action="store_true", help="write baseline from current output")
+ parser.add_argument("--dry-run", action="store_true", help="validate fixture discovery only")
+ parser.add_argument("--pattern", default="", help="fnmatch filter on fixture relative path")
</code_context>
<issue_to_address>
**issue:** Clarify behavior when --write-baseline is used without --baseline-dir.
If `--write-baseline` is passed without `--baseline-dir`, `baseline_dir` ends up falsey and the baseline-writing logic is skipped, so the flag becomes a silent no-op. Consider either enforcing that `--write-baseline` must be paired with a non-empty `--baseline-dir` (fail fast on invalid arg combinations) or defaulting `baseline_dir` to `artifacts_dir` when omitted.
</issue_to_address>
### Comment 2
<location path="docs/OPENSMITH_PARITY.md" line_range="44" />
<code_context>
+- source archive hash
+- nested sample archive scope
+- file extension filters
+- deterministic entry list with size + sha256
+
+The lock file is used to guarantee fixture selection stability across machines
</code_context>
<issue_to_address>
**nitpick (typo):** Consider standard acronym spelling for SHA-256
Here, the algorithm name should be capitalized as “SHA-256” unless you’re referring to a literal identifier.
```suggestion
- deterministic entry list with size + SHA-256
```
</issue_to_address>
### Comment 3
<location path="scripts/opensmith_parity.py" line_range="145" />
<code_context>
run = subprocess.run(command, shell=True, text=True, capture_output=True)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-audit):** Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
*Source: opengrep*
</issue_to_address>
### Comment 4
<location path="scripts/opensmith_parity.py" line_range="145" />
<code_context>
run = subprocess.run(command, shell=True, text=True, capture_output=True)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.dangerous-subprocess-use-tainted-env-args):** Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.
*Source: opengrep*
</issue_to_address>
### Comment 5
<location path="scripts/opensmith_parity.py" line_range="145" />
<code_context>
run = subprocess.run(command, shell=True, text=True, capture_output=True)
</code_context>
<issue_to_address>
**security (python.lang.security.audit.subprocess-shell-true):** Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
```suggestion
run = subprocess.run(command, shell=False, text=True, capture_output=True)
```
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| parser.add_argument("--baseline-dir", default="", help="golden baseline output directory") | ||
| parser.add_argument("--write-baseline", action="store_true", help="write baseline from current output") |
There was a problem hiding this comment.
issue: Clarify behavior when --write-baseline is used without --baseline-dir.
If --write-baseline is passed without --baseline-dir, baseline_dir ends up falsey and the baseline-writing logic is skipped, so the flag becomes a silent no-op. Consider either enforcing that --write-baseline must be paired with a non-empty --baseline-dir (fail fast on invalid arg combinations) or defaulting baseline_dir to artifacts_dir when omitted.
| - source archive hash | ||
| - nested sample archive scope | ||
| - file extension filters | ||
| - deterministic entry list with size + sha256 |
There was a problem hiding this comment.
nitpick (typo): Consider standard acronym spelling for SHA-256
Here, the algorithm name should be capitalized as “SHA-256” unless you’re referring to a literal identifier.
| - deterministic entry list with size + sha256 | |
| - deterministic entry list with size + SHA-256 |
| for fixture in fixtures: | ||
| rel = PurePosixPath(fixture["rel"]) | ||
| command = build_command(args.engine, fixture["input"]) | ||
| run = subprocess.run(command, shell=True, text=True, capture_output=True) |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-audit): Detected subprocess function 'run' without a static string. If this data can be controlled by a malicious actor, it may be an instance of command injection. Audit the use of this call to ensure it is not controllable by an external resource. You may consider using 'shlex.escape()'.
Source: opengrep
| for fixture in fixtures: | ||
| rel = PurePosixPath(fixture["rel"]) | ||
| command = build_command(args.engine, fixture["input"]) | ||
| run = subprocess.run(command, shell=True, text=True, capture_output=True) |
There was a problem hiding this comment.
security (python.lang.security.audit.dangerous-subprocess-use-tainted-env-args): Detected subprocess function 'run' with user controlled data. A malicious actor could leverage this to perform command injection. You may consider using 'shlex.quote()'.
Source: opengrep
| for fixture in fixtures: | ||
| rel = PurePosixPath(fixture["rel"]) | ||
| command = build_command(args.engine, fixture["input"]) | ||
| run = subprocess.run(command, shell=True, text=True, capture_output=True) |
There was a problem hiding this comment.
security (python.lang.security.audit.subprocess-shell-true): Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
| run = subprocess.run(command, shell=True, text=True, capture_output=True) | |
| run = subprocess.run(command, shell=False, text=True, capture_output=True) |
Source: opengrep
![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: 609be288fd
ℹ️ 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".
| p = PurePosixPath(path) | ||
| if p.is_absolute(): | ||
| return False | ||
| if any(part == ".." for part in p.parts): |
There was a problem hiding this comment.
The path safety guard only checks PurePosixPath parts for .., so a lock entry like ..\\escape.txt is treated as a single safe segment and passes validation; during extraction, Windows path handling interprets backslashes as separators, so out_dir.joinpath(...) can resolve outside the corpus directory. This makes crafted lock/zip inputs capable of writing files outside the intended output root on Windows.
Useful? React with 👍 / 👎.
| quoted_input = shlex.quote(input_path) | ||
| if "{input}" in engine: | ||
| return engine.replace("{input}", quoted_input) |
There was a problem hiding this comment.
build_command() relies on shlex.quote() and later executes with shell=True; shlex.quote produces POSIX-style single-quoted arguments, which cmd.exe does not parse as quoting. In Windows environments where fixture paths contain spaces (for example under C:\Users\First Last\...), engine mode receives a broken command line and fails even though the engine command is otherwise valid.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Adds an OpenSmith reverse-engineering/parity scaffold to make corpus selection deterministic and enable repeatable fixture discovery/runs against Generator-85.zip.
Changes:
- Introduces deterministic corpus inventory + extraction tooling (
scripts/opensmith_corpus.py) and commits a corresponding lock file. - Adds a parity harness scaffold with dry-run, engine execution, and baseline plumbing (
scripts/opensmith_parity.py). - Wires new
make opensmith-*targets and adds supporting docs/README updates.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
specs/testing/opensmith/corpus.lock.json |
Adds a committed, deterministic inventory lock (hashes/sizes) for fixture selection. |
specs/testing/opensmith/README.md |
Documents the corpus lock/extract/parity workflow for PR1. |
scripts/opensmith_parity.py |
Adds parity harness scaffold (dry-run + optional engine execution + baseline compare). |
scripts/opensmith_corpus.py |
Adds deterministic inventory/verify/extract tooling for OpenSmith fixture corpora. |
docs/OPENSMITH_PARITY.md |
Adds PR1 parity harness documentation and outputs layout. |
SPEC_TYPES.md |
Registers planned OpenSmith-related spec types (.cst/.csp/.csmap). |
README.md |
Surfaces new OpenSmith commands and links parity doc. |
Makefile |
Adds OpenSmith variables/targets and a PYTHON variable for invoking the new scripts. |
| # For native builds: make CC=cc | ||
| CC ?= cc | ||
| CFLAGS := -O2 -Wall -Werror -std=c11 -Wno-stringop-truncation | ||
| PYTHON ?= python |
There was a problem hiding this comment.
PYTHON ?= python can invoke Python 2 or fail on systems where only python3 is installed. Since the new OpenSmith scripts require Python 3 (f-strings/type annotations and #!/usr/bin/env python3), default PYTHON to python3 (still allowing override), or add a small version check/error message so failures are clearer.
| PYTHON ?= python | |
| PYTHON ?= python3 |
| verify: tools | ||
| @./scripts/regen-all.sh --verify | ||
|
|
||
| opensmith-corpus-lock: |
There was a problem hiding this comment.
opensmith-corpus-lock always runs the inventory subcommand and overwrites the committed lock file. This makes it easy to accidentally clobber specs/testing/opensmith/corpus.lock.json with a lock generated from a different Generator-85.zip (different hash) and can dirty the working tree on every run. Consider making the default behavior verify (fail fast if the local zip doesn't match the committed lock) and adding a separate *-lock-update target for intentional regeneration, or only running inventory when the lock file is missing.
| opensmith-corpus-lock: | |
| opensmith-corpus-lock: | |
| @if [ -f "$(OPENSMITH_LOCK)" ]; then \ | |
| $(PYTHON) ./scripts/opensmith_corpus.py verify \ | |
| --zip "$(OPENSMITH_ZIP)" \ | |
| --lock "$(OPENSMITH_LOCK)"; \ | |
| else \ | |
| $(PYTHON) ./scripts/opensmith_corpus.py inventory \ | |
| --zip "$(OPENSMITH_ZIP)" \ | |
| --lock "$(OPENSMITH_LOCK)"; \ | |
| fi | |
| opensmith-corpus-lock-update: |
| if candidate.exists(): | ||
| return candidate.resolve() | ||
|
|
||
| return path.resolve() |
There was a problem hiding this comment.
If the provided zip path doesn't exist (and none of the Windows fallback candidates exist), resolve_existing_path() still returns path.resolve(), which later produces an unhandled FileNotFoundError/stack trace from zipfile.ZipFile. Consider explicitly raising a FileNotFoundError (or argparse error) here when the path doesn't exist, so callers get a clear, actionable message.
| return path.resolve() | |
| raise FileNotFoundError(f"Path does not exist: {raw}") |
| def build_command(engine: str, input_path: str) -> str: | ||
| quoted_input = shlex.quote(input_path) | ||
| if "{input}" in engine: | ||
| return engine.replace("{input}", quoted_input) | ||
| return f"{engine} {quoted_input}" | ||
|
|
There was a problem hiding this comment.
build_command() uses shlex.quote(), which is POSIX-shell escaping. When combined with subprocess.run(..., shell=True), this will not quote correctly on Windows (cmd.exe doesn't treat single quotes as quoting), and paths containing spaces/unicode may break. If Windows support is desired, prefer building an argv list and running with shell=False, or implement platform-specific quoting/engine parsing (and document any POSIX-shell requirement).
| for fixture in fixtures: | ||
| rel = PurePosixPath(fixture["rel"]) | ||
| command = build_command(args.engine, fixture["input"]) | ||
| run = subprocess.run(command, shell=True, text=True, capture_output=True) |
There was a problem hiding this comment.
Running the engine via subprocess.run(..., shell=True) executes through a shell and makes command construction/escaping fragile, and it increases the blast radius if ENGINE is ever influenced by untrusted input. Consider switching to shell=False with an explicit argv list (e.g., parse --engine into args and splice the input path as a separate argument), or clearly document that --engine is treated as a shell snippet and is unsafe by design.
| run = subprocess.run(command, shell=True, text=True, capture_output=True) | |
| # Normalize command to an argv list to avoid shell=True | |
| if isinstance(command, str): | |
| argv = shlex.split(command) | |
| else: | |
| argv = list(command) | |
| run = subprocess.run(argv, text=True, capture_output=True) |
| make opensmith-corpus-lock OPENSMITH_ZIP="$HOME/Downloads/Generator-85.zip" | ||
| make opensmith-corpus OPENSMITH_ZIP="$HOME/Downloads/Generator-85.zip" | ||
| make opensmith-parity OPENSMITH_ZIP="$HOME/Downloads/Generator-85.zip" |
There was a problem hiding this comment.
The new OpenSmith make opensmith-* commands are listed without context alongside core build/test commands. Since they require Python 3 and a locally provided Generator-85.zip (not in-repo), consider adding an inline note here (or a short sentence above the block) marking them as optional and calling out the prerequisites, to avoid confusing first-time users.
| ## Commands | ||
|
|
||
| ```bash | ||
| make opensmith-corpus-lock OPENSMITH_ZIP="$HOME/Downloads/Generator-85.zip" | ||
| make opensmith-corpus OPENSMITH_ZIP="$HOME/Downloads/Generator-85.zip" | ||
| make opensmith-parity OPENSMITH_ZIP="$HOME/Downloads/Generator-85.zip" | ||
| ``` |
There was a problem hiding this comment.
This doc introduces make opensmith-* targets but doesn't mention that they rely on the Python scripts in scripts/ (Python 3 required). Adding a short prerequisites note near the top (e.g., Python 3 + local Generator-85.zip) would make setup failures easier to diagnose.
| This directory stores the deterministic corpus lock for reverse-engineering | ||
| the `Generator-85.zip` template ecosystem. | ||
|
|
||
| ## Why a lock file? | ||
|
|
||
| - Keeps fixture discovery deterministic across machines. | ||
| - Avoids committing proprietary binaries or full archive dumps. | ||
| - Enables reproducible parity runs against the same sample set. |
There was a problem hiding this comment.
The workflow examples rely on the scripts/opensmith_*.py helpers (Python 3). Consider adding a brief prerequisites note (Python 3 + local Generator-85.zip) near the top so users understand what they need before running the make opensmith-* commands.
| @$(PYTHON) ./scripts/opensmith_corpus.py inventory \ | ||
| --zip "$(OPENSMITH_ZIP)" \ | ||
| --lock "$(OPENSMITH_LOCK)" | ||
|
|
||
| opensmith-corpus: opensmith-corpus-lock | ||
| @$(PYTHON) ./scripts/opensmith_corpus.py extract \ | ||
| --zip "$(OPENSMITH_ZIP)" \ | ||
| --lock "$(OPENSMITH_LOCK)" \ | ||
| --out-dir "$(OPENSMITH_CORPUS_DIR)" | ||
|
|
||
| opensmith-parity: opensmith-corpus | ||
| @if [ -n "$(ENGINE)" ]; then \ |
There was a problem hiding this comment.
The opensmith-corpus target invokes scripts/opensmith_corpus.py extract to write files based on member names taken directly from Generator-85.zip, but the extractor’s path-safety check in that script only handles POSIX-style / paths and does not correctly reject Windows drive letters or backslash-based .. segments. A malicious or compromised zip could include entries like C:\Windows\system32\evil.dll or ..\..\Windows\system32\evil.dll that bypass the check yet cause files to be written outside the intended corpus directory when this Make target is run. The extractor should validate member paths with OS-appropriate semantics (including backslashes and drive letters) and enforce that the final destination always resides under a trusted root before writing.
| @$(PYTHON) ./scripts/opensmith_corpus.py inventory \ | |
| --zip "$(OPENSMITH_ZIP)" \ | |
| --lock "$(OPENSMITH_LOCK)" | |
| opensmith-corpus: opensmith-corpus-lock | |
| @$(PYTHON) ./scripts/opensmith_corpus.py extract \ | |
| --zip "$(OPENSMITH_ZIP)" \ | |
| --lock "$(OPENSMITH_LOCK)" \ | |
| --out-dir "$(OPENSMITH_CORPUS_DIR)" | |
| opensmith-parity: opensmith-corpus | |
| @if [ -n "$(ENGINE)" ]; then \ | |
| @if [ "$(OS)" = "Windows_NT" ]; then \ | |
| echo "ERROR: opensmith-corpus-lock is disabled on Windows for security reasons."; \ | |
| echo " Run this target on a POSIX environment (Linux/macOS/BSD)."; \ | |
| exit 1; \ | |
| fi; \ | |
| $(PYTHON) ./scripts/opensmith_corpus.py inventory \ | |
| --zip "$(OPENSMITH_ZIP)" \ | |
| --lock "$(OPENSMITH_LOCK)" | |
| opensmith-corpus: opensmith-corpus-lock | |
| @if [ "$(OS)" = "Windows_NT" ]; then \ | |
| echo "ERROR: opensmith-corpus is disabled on Windows for security reasons."; \ | |
| echo " Run this target on a POSIX environment (Linux/macOS/BSD)."; \ | |
| exit 1; \ | |
| fi; \ | |
| $(PYTHON) ./scripts/opensmith_corpus.py extract \ | |
| --zip "$(OPENSMITH_ZIP)" \ | |
| --lock "$(OPENSMITH_LOCK)" \ | |
| --out-dir "$(OPENSMITH_CORPUS_DIR)" | |
| opensmith-parity: opensmith-corpus | |
| @if [ "$(OS)" = "Windows_NT" ]; then \ | |
| echo "ERROR: opensmith-parity is disabled on Windows for security reasons."; \ | |
| echo " Run this target on a POSIX environment (Linux/macOS/BSD)."; \ | |
| exit 1; \ | |
| fi; \ | |
| if [ -n "$(ENGINE)" ]; then \ |
| opensmith-parity: opensmith-corpus | ||
| @if [ -n "$(ENGINE)" ]; then \ | ||
| $(PYTHON) ./scripts/opensmith_parity.py \ | ||
| --inventory "$(OPENSMITH_LOCK)" \ | ||
| --corpus-dir "$(OPENSMITH_CORPUS_DIR)" \ | ||
| --artifacts-dir "$(OPENSMITH_PARITY_DIR)" \ | ||
| --engine "$(ENGINE)"; \ | ||
| else \ | ||
| $(PYTHON) ./scripts/opensmith_parity.py \ | ||
| --inventory "$(OPENSMITH_LOCK)" \ | ||
| --corpus-dir "$(OPENSMITH_CORPUS_DIR)" \ | ||
| --artifacts-dir "$(OPENSMITH_PARITY_DIR)" \ | ||
| --dry-run; \ | ||
| fi |
There was a problem hiding this comment.
The opensmith-parity target calls scripts/opensmith_parity.py with an unescaped ENGINE string and fixture paths substituted into a shell command that is then executed via subprocess.run(..., shell=True). While fixture paths are run through shlex.quote, that function only provides POSIX shell escaping and does not protect against meta-characters (like & or |) when using cmd.exe on Windows, so a crafted fixture filename from the corpus can inject additional commands into the engine invocation. To mitigate this, avoid shell=True and pass the engine and input as an argument list (or implement OS-appropriate escaping) so untrusted fixture paths cannot influence the shell command line.
There was a problem hiding this comment.
no python, just BDE with models
OpenSmith parity scaffold: deterministic corpus lock/extract tooling, parity harness dry-run plumbing, and Make/docs updates for the PR1 RE workflow.
Summary by Sourcery
Add a deterministic OpenSmith corpus lock and parity harness scaffold wired into the build to support reverse‑engineering workflows against Generator-85.zip.
New Features:
Enhancements:
Documentation:
Chores: