Repository navigation
fix(artifact): escape hostile metadata in CLI reports - #405
Merged
Merged
Conversation
what changed: The validator now exposes manifest state and identity metadata only after schema validation. The artifact CLI escapes control characters in every text diagnostic field, so malformed manifest strings remain on one physical line and cannot forge a verdict. JSON reports retain normal escaping. why: Issue #360 showed package-supplied state, packageId, clientRef and unknown schema keys could inject terminal controls or a second PASS line, including under --quiet. Quiet mode promises a single verdict line and exit status alone cannot protect a human reading output. what it touched: artifact-validator report shape, artifact CLI text rendering, artifact CLI regressions, architecture map, changelog, README test inventory. No format schema or exit-code change. what you verified: verified - red-first 22 passed/2 expected failures; then 69 artifact-cli and artifact-validator tests passed. Coverage includes forged state under --quiet, newline in an unknown manifest key, terminal escape in packageId, newline clientRef, and null metadata for invalid schemas. Architecture-map 21 passed; handoff OK (33 ledger entries, 27 evidence rows); preflight PASS (28 pass, 1 warn, 0 fail); doctor ok with no blocking findings. Cached diff check clean. A retained full gate blocked on the preflight-scope CLI case (1778 passed, one failed, 12 unsupported); a subsequent interrupted focused run returned null child statuses in other CLI cases. The precise cause was not retained by those helpers. An isolated repeat with unchanged assertions passed all 120 CLI/artifact-CLI tests in 396.4 seconds, with no spawn errors. Temporary diagnostics were restored from exact backups before the normal full gate retry. The required full offline suite runs in the commit gate before acceptance. what you got wrong and fixed: mistake - the first green-regression assertion rejected the string PASS anywhere in output, including the escaped key name on its now-safe diagnostic line. where - artifact-cli.test.mjs hostile metadata test. impact - it falsely failed a safe one-line rendering; no product behavior or delivered result was affected. cause - the assertion checked for a substring, not an actual forged physical output line. fix - assert no physical line starts with the forged PASS verdict, and assert the unknown key yields exactly one MANIFEST-SCHEMA line. verified - artifact-cli and artifact-validator groups passed 69 tests, 0 failures.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #360.
what changed: Artifact text reports escape control characters in metadata, diagnostics and remedies, keeping each field on one physical line. Metadata is withheld until the manifest passes schema validation.
why: An invalid package could inject a second quiet verdict or terminal control sequence into the report. Validation now preserves the report's boundaries and does not present invalid declarations as trusted metadata.
what it touched: The artifact validator, artifact CLI, two CLI regressions, the architecture map, changelog and test inventory.
what you verified: verified — the new regressions failed before the fix; 69 focused artifact tests and 21 architecture-map tests passed. An isolated CLI run passed all 120 tests after investigation of a prior full-gate failure. The normal full commit gate subsequently allowed commit 6295179. Handoff, preflight and doctor passed. All seven GitHub CI jobs passed on this exact head: offline-suite run.
what you got wrong and fixed: mistake — the first assertion rejected PASS anywhere in output, including an escaped key on a safe diagnostic line. where — the hostile-metadata artifact CLI regression. impact — it falsely failed safe one-line rendering; no published product behavior was affected. cause — substring matching did not distinguish an actual forged physical output line. fix — assert no physical line starts with the forged verdict and exactly one MANIFEST-SCHEMA line is present. verified — focused artifact and isolated CLI suites passed. The earlier full-gate failure and interrupted child-status investigation remain documented in the commit report; their original cause was not established.