Skip to content

Enforce strict latest theme linkage#38

Closed
Pigbibi wants to merge 2 commits into
mainfrom
codex/slice-b1-linkage-integrity
Closed

Enforce strict latest theme linkage#38
Pigbibi wants to merge 2 commits into
mainfrom
codex/slice-b1-linkage-integrity

Conversation

@Pigbibi

@Pigbibi Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add strict v2 latest-signal validation from the signal-declared theme source
  • reject undeclared theme path overrides and declared SHA-256 mismatches
  • require exact signal/theme as_of equality

Scope

Linkage integrity only. Expiry, UTC reference-date handling, CLI flags, and coverage/data-quality gates are intentionally deferred to B2/C.

Validation

  • python3 -m pytest -q tests/test_signal_validation.py (15 passed)
  • python3 -m pytest -q (45 passed)
  • git diff --check

Research-only scope; no execution, portfolio, account, or production dependency changes.

Co-Authored-By: Codex <noreply@openai.com>
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

🤖 Codex PR Review

🚫 Merge blocked: 2 serious issue(s) found in high-risk files

⚖️ Codex Review Arbitration

🚫 block: src/research_signal_context_pipelines/schema.py still proves both current security findings. First, resolve_source() returns (base_dir / path).resolve() for relative inputs and never checks that the resolved path remains under signal_base_dir, so a declared source like ../../other.json can escape the signal tree while still being accepted if the override matches. Second, when a hash is present the validator computes hashlib.sha256(artifact_path.read_bytes()).hexdigest() and later separately parses json.loads(artifact_path.read_text(...)), so digest verification and schema/as_of validation are performed on two different filesystem reads and can observe different file contents. These issues are independent of the prior blocking finding about falsy source_hashes values; that earlier behavior was addressed by explicit key-presence checks and SHA-256 string validation, so there is no contract conflict.

🚫 Blocking Issues

These issues must be fixed before this PR can be merged:

1. 🟠 [HIGH] Security in src/research_signal_context_pipelines/schema.py

Relative theme sources are resolved with (base_dir / path).resolve() but never checked to stay under signal_base_dir. A v2 signal can therefore declare ../../.../some.json and make the validator read an arbitrary file outside the signal directory tree. That breaks the trust boundary this API is trying to enforce and can bind a signal to unintended local artifacts. (line 177)

Suggestion: After resolving the path, verify that the resolved file is contained within signal_base_dir (for example with resolved.relative_to(base_dir) or an equivalent common-path check) and reject any source that escapes the allowed base directory.

2. 🟠 [HIGH] Security in src/research_signal_context_pipelines/schema.py

The validator reads the theme artifact twice: once with read_bytes() for the SHA-256 check and again with read_text() for JSON parsing and as_of validation. If the file is replaced between those reads, the function can accept the hash of one artifact but validate the contents of a different one, defeating the new strict linkage guarantee under concurrent or adversarial filesystem changes. (line 201)

Suggestion: Read the artifact exactly once into memory, compute the digest from those bytes, and parse the JSON from the same byte buffer so every check applies to one immutable snapshot of the file contents.


Review by Codex PR Review bot • PR

Co-Authored-By: Codex <noreply@openai.com>
@Pigbibi

Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Stopped after the single permitted remediation. The follow-up review still blocks on relative-source path escape outside signal_base_dir and validating SHA-256 plus JSON/as_of from separate file reads. No further changes or merge will be made in this PR; B2/C remain blocked until linkage arbitration is resolved.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e54123ae3d

ℹ️ 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".

Comment on lines +160 to +166
def validate_latest_signal(
payload: Mapping[str, Any],
*,
theme_artifact_path: str | Path | None = None,
) -> None:
"""Validate a v2 signal against its declared theme artifact linkage."""
validate_signal(payload)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Route latest validation through the strict checker

In the contexts I checked, this new strict validator is not used outside the added tests: scripts/validate_latest_signal.py still imports/calls validate_signal, and both .github/workflows/ci.yml and dispatch_shadow_signal.yml invoke that script. A v2 latest signal whose declared theme_momentum_snapshot has a mismatched as_of or bad hash is still reported as valid by the existing validation workflow, so the linkage added here is not actually enforced for the repository's validation path. Please route the latest-signal CLI/workflows, or an equivalent production validation path, through validate_latest_signal while preserving legacy/example handling as needed.

Useful? React with 👍 / 👎.

@Pigbibi

Pigbibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Superseded after the single allowed remediation. Re-slicing from latest main into a library-only strict-linkage validator primitive; CLI/workflow enforcement will follow in a separate PR. No further changes will be made on this PR.

@Pigbibi Pigbibi closed this Jul 13, 2026
@Pigbibi
Pigbibi deleted the codex/slice-b1-linkage-integrity branch July 13, 2026 10:44
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.

1 participant