Skip to content

feat: validate required run artifacts against a schema (--require-artifact-schema) - #1071

Merged
steipete merged 18 commits into
openclaw:mainfrom
dwin-gharibi:feat/require-artifact-schema
Jul 17, 2026
Merged

feat: validate required run artifacts against a schema (--require-artifact-schema)#1071
steipete merged 18 commits into
openclaw:mainfrom
dwin-gharibi:feat/require-artifact-schema

Conversation

@dwin-gharibi

@dwin-gharibi dwin-gharibi commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Closes #1070

Summary

Adds --require-artifact-schema remote=schema.json, a post-command evidence gate that validates a required JSON artifact instead of checking existence alone.

  • Supports standard JSON Schema drafts 4, 6, 7, 2019-09, and 2020-12.
  • Supports bounded local static references and embedded resources; external fetches and active recursive or dynamic references fail closed.
  • Returns exit 2 for malformed or unsafe schemas and exit 7 for missing, oversized, malformed, or non-conforming artifacts.
  • Emits bounded, value-redacted JSON Pointer diagnostics and records results in timing evidence.
  • Documents schema, artifact, reference, regular-expression, work, and diagnostic limits.

Safety and dependency notes

Schema definitions are capped at 1 MiB and artifacts at 5 MiB. JSON depth, value count, numeric work, reference expansion, validation work, regular-expression source/program work, and diagnostic output all have explicit limits. The regular-expression engine uses Go's linear-time RE2 implementation through a fail-closed ECMA-262 compatibility layer.

The implementation uses github.com/santhosh-tekuri/jsonschema/v6 plus github.com/dlclark/regexp2/syntax. A compatibility correction for mixed legacy dependencies resources is proposed upstream at santhosh-tekuri/jsonschema#262; this PR temporarily replaces the module with the public commit steipete/jsonschema@7235d3e. The replacement is source-compatible and can be removed once upstream ships the fix. Both dependencies are active, non-archived projects; no credentials or new runtime configuration are introduced.

Verification

  • go test -race ./...
  • go vet ./...
  • go build -trimpath -o bin/crabbox ./cmd/crabbox
  • go mod verify
  • Worker format, lint, typecheck, build, and 1,059 tests
  • Docs-site build
  • Source-blind local-container behavior contract: matching artifact exit 0; invalid and missing artifacts exit 7; malformed schema exit 2 before provider access; artifact value absent from validation diagnostics
  • Exact-head AWS SSH behavior matrix: matching artifact exit 0; type mismatch exit 7; missing artifact exit 7; 5 MiB plus one byte exit 7; malformed schema exit 2 before run creation; lease released afterward
  • Structured code review: clean, no actionable findings
  • Exact-head hosted CI at b6a830872ecb262ec4259dd1d2c32957e1c8ae5c: all jobs green, including Go race/coverage/build, Worker, release snapshot, Apple VM, Windows cancellation, scripts, connector lifecycles, docs, Docs UI Proof, and Connector E2E Smokes — https://github.com/openclaw/crabbox/actions/runs/29575888071

Contributor credit

Preserves Dwin Gharibi's authorship with co-authored maintainer commits and the changelog credit to @dwin-gharibi.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Jul 13, 2026
@clawsweeper

clawsweeper Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 17, 2026, 7:12 AM ET / 11:12 UTC.

Summary
The PR adds an opt-in --require-artifact-schema post-run gate with bounded standard JSON Schema validation, timing evidence, tests, documentation, and new validator dependencies.

Reproducibility: not applicable. This PR introduces a new opt-in validation mode rather than fixing behavior that violates an existing contract. Its proposed behavior nevertheless has credible SSH-backed runtime proof.

Review metrics: 3 noteworthy metrics.

  • Implementation size: 3,244 additions across 8 files. The feature is substantially larger than the flag wiring and creates a meaningful permanent maintenance surface.
  • Validator coverage: 1,798 implementation lines and 1,353 test lines added. The extensive bounded-validation code and coverage indicate both strong engineering effort and a broad contract to maintain.
  • Dependency surface: 2 modules added, including 1 replace directive. The change introduces supply-chain and update ownership beyond the existing CLI implementation.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #1070
Summary: This PR is the concrete implementation candidate for the linked artifact-schema feature request.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Obtain maintainer approval for the permanent schema-compatibility and fork-ownership contract.

Risk before merge

  • [P1] Merging establishes a broad JSON Schema compatibility and maintenance contract whose future semantics may affect repositories that adopt the new flag.
  • [P1] All Crabbox builds would depend on a repository-specific fork pinned through a pseudo-version replacement, requiring explicit long-term supply-chain, security-update, and upstream-sync ownership.

Maintainer options:

  1. Stabilize before merge (recommended)
    Retain the feature but replace the provisional dependency arrangement with a maintainer-approved upstream or explicitly owned fork contract.
  2. Explicitly own the fork
    Accept the implementation as written and assign responsibility for fork review, security updates, upstream synchronization, and schema compatibility.
  3. Pause the schema engine
    Close or defer the PR if maintainers do not want Crabbox to become a JSON Schema implementation surface.

Next step before merge

  • [P2] A maintainer must choose the permanent schema-validation scope and dependency ownership; no narrow correctness repair remains for an automated worker.

Maintainer decision needed

  • Question: Should Crabbox permanently provide --require-artifact-schema through the proposed fork-backed standard JSON Schema implementation, narrow the schema contract, or leave schema enforcement to each repository's run command?
  • Rationale: Automated review can assess correctness and proof, but it cannot decide whether Crabbox should own this substantial standards, compatibility, and dependency-maintenance surface.
  • Likely owner: steipete — They performed the latest implementation, security-hardening, and live-proof passes and explicitly framed the unresolved product decision.
  • Options:
    • Stabilize the dependency path (recommended): Keep the opt-in feature but first establish a maintained upstream release or an explicitly owned first-party fork and document the supported schema compatibility contract.
    • Accept the current contract: Merge the current standards-compatible implementation while explicitly accepting ownership of the fork, resource bounds, and future schema compatibility.
    • Keep validation repo-owned: Do not add a schema engine to Crabbox and continue requiring repositories to validate evidence inside their executed commands.

Security
Needs attention: The bounded implementation avoids obvious artifact-disclosure concerns, but the forked schema dependency requires explicit supply-chain ownership before merge.

Review details

Best possible solution:

Retain the opt-in evidence gate only after maintainers approve a standards-compatible validator with explicit bounded-execution guarantees and a sustainable upstream or first-party dependency ownership plan.

Do we have a high-confidence way to reproduce the issue?

Not applicable: this PR introduces a new opt-in validation mode rather than fixing behavior that violates an existing contract. Its proposed behavior nevertheless has credible SSH-backed runtime proof.

Is this the best way to solve the issue?

Unclear: the implementation appears correct and carefully bounded, but maintainers must decide whether the broad fork-backed schema contract is the narrowest sustainable product solution.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 8c40b42f42d6.

Label changes

Label justifications:

  • P3: This is an optional new evidence-validation capability, not a regression or blocked existing workflow.
  • merge-risk: 🚨 other: Merging commits Crabbox to a substantial schema-engine and fork-maintenance contract that correctness tests alone cannot settle.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (terminal): After-fix terminal evidence shows the branch-built CLI running through a localhost SSH target for valid, violating, missing, unparseable, malformed-schema, and oversized cases with the expected results.
  • proof: sufficient: Contributor real behavior proof is sufficient. After-fix terminal evidence shows the branch-built CLI running through a localhost SSH target for valid, violating, missing, unparseable, malformed-schema, and oversized cases with the expected results.
Evidence reviewed

Security concerns:

  • [medium] Approve ownership of the forked schema dependency — go.mod:51
    The branch replaces the upstream schema module with github.com/steipete/jsonschema/v6 at a pseudo-version, making Crabbox responsible for reviewing, securing, updating, and synchronizing that fork rather than consuming a stable upstream release.
    Confidence: 0.98

What I checked:

  • New CLI and evidence surface: The branch registers and executes the repeatable artifact-schema flag and adds schema results to timing reports; this capability is absent from current main. (internal/cli/run.go:250, b6a830872ecb)
  • Large validation contract: The proposed validator is a 1,798-line implementation supporting multiple standard JSON Schema drafts with explicit resource and work limits, making this a substantial permanent product surface rather than a narrow bug fix. (internal/cli/run_artifact_schema.go:1, b6a830872ecb)
  • Supply-chain change: The branch adds regexp2 and replaces the upstream JSON Schema module with a same-day github.com/steipete/jsonschema/v6 pseudo-version. (go.mod:51, b6a830872ecb)
  • Maintainer-intent evidence: The July 17, 2026 correctness-pass comment says the implementation is technically merge-ready but must not merge until maintainers choose whether Crabbox should own this contract, use a maintained standards-compatible validator, or keep validation repository-owned. (d04a148cb46e)
  • Real behavior proof: The same review comment records a branch-built localhost SSH run covering valid, violating, missing, unparseable, malformed-schema, and oversized artifacts with the expected exit codes, plus successful build, vet, focused tests, and autoreview. (internal/cli/run_artifact_schema_test.go:1, d04a148cb46e)
  • Linked canonical request: The PR uses closing syntax for the open feature request at Validate required run artifacts against a schema (--require-artifact-schema) #1070, so the issue should remain open until this implementation or another approved solution lands. (b6a830872ecb)

Likely related people:

  • steipete: Authored the major hardening, standards-compatible validator integration, live CLI proof, dependency fork, and latest branch maintenance commits, then explicitly identified the remaining product choice. (role: recent area contributor and product-decision participant; confidence: high; commits: 248b5c125b92, 3ff3ca8717c3, d04a148cb46e; files: internal/cli/run_artifact_schema.go, internal/cli/run_artifact_schema_test.go, internal/cli/run.go)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (9 earlier review cycles; latest 8 shown)
  • reviewed 2026-07-14T11:38:42.488Z sha 4929212 :: needs real behavior proof before merge. :: [P2] Reject content after the schema JSON value
  • reviewed 2026-07-14T11:58:24.224Z sha c7fea79 :: needs real behavior proof before merge. :: [P2] Reject content after the schema JSON value
  • reviewed 2026-07-14T12:22:58.866Z sha c7fea79 :: needs real behavior proof before merge. :: [P2] Reject trailing content in schema files
  • reviewed 2026-07-14T12:50:04.844Z sha c7fea79 :: needs real behavior proof before merge. :: [P2] Reject trailing content after the schema value
  • reviewed 2026-07-14T13:16:45.235Z sha c7fea79 :: needs real behavior proof before merge. :: [P2] Reject trailing content after the schema JSON value
  • reviewed 2026-07-16T08:49:56.258Z sha 781dc31 :: needs real behavior proof before merge. :: [P2] Reject trailing content after the schema JSON value
  • reviewed 2026-07-17T01:46:20.563Z sha 3ff3ca8 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-17T03:46:27.932Z sha d04a148 :: needs maintainer review before merge. :: none

@dwin-gharibi

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Command router queued. I will update this comment with the next step.

@clawsweeper clawsweeper Bot added merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. and removed merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. merge-risk: 🚨 availability 🚨 Merging this PR could cause crashes, hangs, restart loops, stalls, or process outages. labels Jul 14, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 14, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels Jul 16, 2026
steipete and others added 2 commits July 17, 2026 02:13
Co-authored-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
Co-authored-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
@steipete

Copy link
Copy Markdown
Contributor

Maintainer improvement pass pushed to the contributor branch at 3ff3ca87.

Improvements:

  • Made schema parsing genuinely fail-closed: exactly one JSON object, no trailing content, duplicate object names, null/invalid keyword shapes, duplicate required entries, duplicate enum values, or unsupported keywords.
  • Preserved exact JSON number semantics, including values beyond float64 precision and compact numbers with very large exponents.
  • Removed rejected artifact values from diagnostics, capped retained/printed violations, and added an explicit truncation marker.
  • Fixed native Windows short-read handling by reading until EOF or the byte limit.
  • Restricted the artifact operand to one exact safe relative path, including rejecting glob and Windows drive/ADS syntax.
  • Added regression coverage for all of the above while preserving the provider-neutral core/SSH-target boundary.

Proof: go build ./..., go vet ./..., focused tests, focused race tests, and docs-site generation pass. The full race suite passed the changed CLI package and all relevant packages; one unrelated timing-sensitive external-provider test missed its bound under suite load and passed immediately when retried alone with race.

Final Codex autoreview command: .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main. Verdict: clean, no accepted/actionable findings, overall confidence 0.91.

Remaining gate — do not merge: a maintainer product decision is still required on whether Crabbox should own this bespoke JSON-Schema-subset contract at all, versus narrowing the feature or using a maintained standards-compatible approach. If the feature is retained, it still needs redacted real SSH-backed CLI proof for valid, invalid, missing, oversized, and malformed-schema cases. This fork also needs maintainer approval before hosted workflows can run.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jul 17, 2026
@steipete

Copy link
Copy Markdown
Contributor

Maintainer correctness pass pushed to the contributor branch at d04a148c (after merging current origin/main).

Two reproducible P2 fail-closed bugs found by live testing are fixed:

  1. Schema keyword matching was case-insensitive through Go's JSON struct decoder. A later "REQUIRED": [] could alias and overwrite a real "required": ["proof"] constraint. Schema nodes now use an exact, case-sensitive keyword allowlist. Unknown or mis-cased keywords fail preflight with exit 2, consistent with this tool's documented fail-closed subset.
  2. Go's JSON decoder replaces invalid UTF-8 with U+FFFD. Raw schema and artifact bytes are now checked before decoding. Invalid UTF-8 schema input fails preflight with exit 2; invalid UTF-8 artifact input fails validation with exit 7.

Regression provenance:

BEFORE
mis-cased required: expected invalid schema keyword shape, got no error
invalid UTF-8 schema: expected exit 2, got nil
invalid UTF-8 artifact: expected exit 7, got nil

AFTER
go test ./internal/cli -run 'TestParseArtifactSchemaRejectsInvalidKeywordShapes|TestLoadRequireArtifactSchemas|TestValidateArtifactSchemasWithReaderBehaviour' -count=1
ok github.com/openclaw/crabbox/internal/cli

Real CLI proof through a localhost SSH target, using the branch-built binary:

valid             -> 0  schema valid.json: ok
violation         -> 7  schema violation.json: failed 1 check
missing           -> 7  schema missing.json: fetch failed
unparseable       -> 7  schema unparseable.json: failed 1 check
malformed-schema  -> 2  invalid schema: EOF
oversized         -> 7  artifact exceeds the 5242880-byte validation limit

Verification clean:

go build -trimpath -o bin/crabbox ./cmd/crabbox
go vet ./...
go test ./internal/cli -run 'ArtifactSchema|ValidateJSONAgainstSchema|DecodeBoundedBase64|RemoteBoundedReadBase64' -count=1
.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main
autoreview clean: no accepted/actionable findings reported (confidence 0.92)

Technically merge-ready. Still do not merge until the maintainer decides whether Crabbox should own this bespoke JSON-Schema-subset contract, use a maintained standards-compatible validator, or keep validation repo-owned. This pass intentionally does not make that product decision.

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 17, 2026
steipete and others added 3 commits July 17, 2026 04:02
Co-authored-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
Co-authored-by: Dwin Gharibi <dwin.gharibi@email.kntu.ac.ir>
@steipete
steipete merged commit cf5081f into openclaw:main Jul 17, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P3 Low-risk cleanup, docs, polish, ergonomics, or speculative feature. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Validate required run artifacts against a schema (--require-artifact-schema)

2 participants