From e4f41593d4cf940f391f8d5325aec1539209fe0b Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 24 Jun 2025 22:45:38 -0600 Subject: [PATCH] chore(ci): fix test path, remove an action (#971) --- .github/workflows/test-output.yml | 20 +++++++--------- crates/zizmor/src/models/action.rs | 3 +-- crates/zizmor/src/models/workflow.rs | 3 +-- crates/zizmor/src/output/sarif.rs | 23 +++++++++++-------- crates/zizmor/src/registry.rs | 19 +++++++++++---- crates/zizmor/tests/integration/e2e.rs | 14 +++++++++++ ...tion__e2e__invalid_input_not_strict-2.snap | 7 ++++++ ...ration__e2e__invalid_input_not_strict.snap | 7 ++++++ .../integration__e2e__invalid_inputs-10.snap | 7 +++--- .../integration__e2e__invalid_inputs-2.snap | 7 +++--- .../integration__e2e__invalid_inputs-3.snap | 7 +++--- .../integration__e2e__invalid_inputs-4.snap | 7 +++--- .../integration__e2e__invalid_inputs-5.snap | 7 +++--- .../integration__e2e__invalid_inputs-6.snap | 7 +++--- .../integration__e2e__invalid_inputs-7.snap | 7 +++--- .../integration__e2e__invalid_inputs-8.snap | 7 +++--- .../integration__e2e__invalid_inputs-9.snap | 7 +++--- .../integration__e2e__invalid_inputs.snap | 7 +++--- 18 files changed, 96 insertions(+), 70 deletions(-) create mode 100644 crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict-2.snap create mode 100644 crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict.snap diff --git a/.github/workflows/test-output.yml b/.github/workflows/test-output.yml index 586892fc..0002ca3f 100644 --- a/.github/workflows/test-output.yml +++ b/.github/workflows/test-output.yml @@ -36,17 +36,13 @@ jobs: category: zizmor-test-sarif-presentation - name: Leave comment - uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 - with: - script: | - let url = `https://github.com/zizmorcore/zizmor/security/code-scanning?query=pr%3A${context.issue.number}+is%3Aopen+sort%3Acreated-desc` - - github.rest.issues.createComment({ - issue_number: context.issue.number, - owner: context.repo.owner, - repo: context.repo.repo, - body: `:robot: Presentation results: <${url}>` - }) + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + PR_NUMBER: ${{ github.event.pull_request.number }} + URL: "https://github.com/zizmorcore/zizmor/security/code-scanning?query=pr%3A${{ github.event.pull_request.number }}+is%3Aopen+sort%3Acreated-desc" + run: | + gh pr comment "${PR_NUMBER}" \ + --body ":robot: SARIF results: ${URL}" test-github-presentation: name: Test GitHub annotations presentation @@ -70,4 +66,4 @@ jobs: -- \ --no-exit-codes \ --format github \ - tests/integration/test-data/several-vulnerabilities.yml + crates/zizmor/tests/integration/test-data/several-vulnerabilities.yml diff --git a/crates/zizmor/src/models/action.rs b/crates/zizmor/src/models/action.rs index 0a3346ec..a8a70629 100644 --- a/crates/zizmor/src/models/action.rs +++ b/crates/zizmor/src/models/action.rs @@ -61,8 +61,7 @@ impl HasInputs for Action { impl Action { /// Load an action from a buffer, with an assigned name. pub(crate) fn from_string(contents: String, key: InputKey) -> Result { - let inner = from_str_with_validation(&contents, &ACTION_VALIDATOR) - .with_context(|| format!("failed to load action from {key}"))?; + let inner = from_str_with_validation(&contents, &ACTION_VALIDATOR)?; let document = yamlpath::Document::new(&contents) .context("failed to load internal pathing document")?; diff --git a/crates/zizmor/src/models/workflow.rs b/crates/zizmor/src/models/workflow.rs index b6c626b2..323110eb 100644 --- a/crates/zizmor/src/models/workflow.rs +++ b/crates/zizmor/src/models/workflow.rs @@ -121,8 +121,7 @@ impl HasInputs for Workflow { impl Workflow { /// Load a workflow from a buffer, with an assigned name. pub(crate) fn from_string(contents: String, key: InputKey) -> Result { - let inner = from_str_with_validation(&contents, &WORKFLOW_VALIDATOR) - .with_context(|| format!("failed to load workflow from {key}"))?; + let inner = from_str_with_validation(&contents, &WORKFLOW_VALIDATOR)?; let document = yamlpath::Document::new(&contents) .context("failed to load internal pathing document")?; diff --git a/crates/zizmor/src/output/sarif.rs b/crates/zizmor/src/output/sarif.rs index df0b286a..8a75b7ae 100644 --- a/crates/zizmor/src/output/sarif.rs +++ b/crates/zizmor/src/output/sarif.rs @@ -102,16 +102,21 @@ fn build_result(finding: &Finding<'_>) -> SarifResult { SarifResult::builder() .rule_id(format!("zizmor/{id}", id = finding.ident)) - // NOTE: We use the primary location's annotation for the result's message. - // This is conceptually incorrect since the location's annotation should - // only be on the location itself. However, GitHub's SARIF viewer does not - // render location-level messages, so we use the primary location's message - // to ensure something reasonable is presented. - // This ends up being OK since the only other thing we'd put here - // is the finding's description, which is already in the rule's help message. - // See https://github.com/zizmorcore/zizmor/issues/526 for context. - .message(&primary.symbolic.annotation) + // NOTE: Between 1.4.0 and 1.9.0 we used the primary location's + // annotation for the message here. This produced a _slightly_ + // nicer message in some cases, but also produced meaningless + // code security alert titles when the primary annotation was + // terse. So now we use the finding's description again, like + // we did before 1.4.0. + .message(finding.desc) .locations(build_locations(std::iter::once(primary))) + // TODO: Evaluate including the related locations via CodeFlows + // instead -- GitHub seems to do a better job of rendering these, + // but still doesn't do a great job of putting all of the locations + // into the same render. + // TODO: Give related locations IDs and back-link to them in the + // main location's message -- GitHub renders these as modals that + // users can click through to see more context. .related_locations(build_locations( finding .visible_locations() diff --git a/crates/zizmor/src/registry.rs b/crates/zizmor/src/registry.rs index 045a6964..132f019f 100644 --- a/crates/zizmor/src/registry.rs +++ b/crates/zizmor/src/registry.rs @@ -54,6 +54,15 @@ pub(crate) enum InputKind { Action, } +impl std::fmt::Display for InputKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + InputKind::Workflow => write!(f, "workflow"), + InputKind::Action => write!(f, "action"), + } + } +} + #[derive(Debug, Clone, Eq, Hash, PartialEq, Serialize, PartialOrd, Ord)] pub(crate) struct LocalKey { /// The path's nondeterministic prefix, if any. @@ -204,11 +213,11 @@ impl InputRegistry { contents: String, key: InputKey, ) -> anyhow::Result<()> { - tracing::debug!("registering {kind:?} input as with key {key}"); + tracing::debug!("registering {kind} input as with key {key}"); let input: Result = match kind { - InputKind::Workflow => Workflow::from_string(contents, key).map(|wf| wf.into()), - InputKind::Action => Action::from_string(contents, key).map(|a| a.into()), + InputKind::Workflow => Workflow::from_string(contents, key.clone()).map(|wf| wf.into()), + InputKind::Action => Action::from_string(contents, key.clone()).map(|a| a.into()), }; match input { @@ -218,10 +227,10 @@ impl InputRegistry { Ok(()) } Err(e @ InputError::Schema { .. }) if !self.strict => { - tracing::warn!("failed to validate input as {kind:?}: {e}"); + tracing::warn!("failed to validate input as {kind}: {e}"); Ok(()) } - Err(e) => Err(anyhow!(e)).with_context(|| format!("failed to load input as {kind:?}")), + Err(e) => Err(anyhow!(e)).with_context(|| format!("failed to load {key} as {kind}")), } } diff --git a/crates/zizmor/tests/integration/e2e.rs b/crates/zizmor/tests/integration/e2e.rs index 97efbe12..795f1c11 100644 --- a/crates/zizmor/tests/integration/e2e.rs +++ b/crates/zizmor/tests/integration/e2e.rs @@ -237,6 +237,20 @@ fn invalid_inputs() -> Result<()> { Ok(()) } +#[test] +fn invalid_input_not_strict() -> Result<()> { + for tc in ["invalid-workflow", "invalid-action-1/action"] { + insta::assert_snapshot!( + zizmor() + .expects_failure(true) + .input(input_under_test(&format!("invalid/{tc}.yml"))) + .run()? + ); + } + + Ok(()) +} + #[test] fn pr_960_backstop() -> Result<()> { // Backstop test for PR #960. diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict-2.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict-2.snap new file mode 100644 index 00000000..ef11b0f1 --- /dev/null +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict-2.snap @@ -0,0 +1,7 @@ +--- +source: crates/zizmor/tests/integration/e2e.rs +expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{tc}.yml\"))).run()?" +--- + WARN collect_inputs: zizmor::registry: failed to validate input as action: input does not match expected validation schema +fatal: no audit was performed +no inputs collected diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict.snap new file mode 100644 index 00000000..24c0f1b5 --- /dev/null +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_input_not_strict.snap @@ -0,0 +1,7 @@ +--- +source: crates/zizmor/tests/integration/e2e.rs +expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{tc}.yml\"))).run()?" +--- + WARN collect_inputs: zizmor::registry: failed to validate input as workflow: input does not match expected validation schema +fatal: no audit was performed +no inputs collected diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-10.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-10.snap index 95e3141c..d1ed99ed 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-10.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-10.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Action +failed to load file://@@INPUT@@ as action Caused by: - 0: failed to load action from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: null is not of type "object" + 0: input does not match expected validation schema + 1: null is not of type "object" diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-2.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-2.snap index edefc6c6..bd8fa0a2 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-2.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-2.snap @@ -3,10 +3,9 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: on.workflow_call.inputs.input: "type" is a required property + 0: input does not match expected validation schema + 1: on.workflow_call.inputs.input: "type" is a required property Additional properties are not allowed ('boom' was unexpected) diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-3.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-3.snap index a56a309c..a30b6842 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-3.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-3.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: null is not of type "object" + 0: input does not match expected validation schema + 1: null is not of type "object" diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-4.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-4.snap index fb7fb6e6..5b82e4e3 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-4.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-4.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: "lol" is not of type "object" + 0: input does not match expected validation schema + 1: "lol" is not of type "object" diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-5.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-5.snap index 3f07ac89..4af48921 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-5.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-5.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: invalid YAML syntax: mapping values are not allowed in this context at line 3 column 8 - 2: mapping values are not allowed in this context at line 3 column 8 + 0: invalid YAML syntax: mapping values are not allowed in this context at line 3 column 8 + 1: mapping values are not allowed in this context at line 3 column 8 diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-6.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-6.snap index a56a309c..a30b6842 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-6.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-6.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: null is not of type "object" + 0: input does not match expected validation schema + 1: null is not of type "object" diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-7.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-7.snap index a56a309c..a30b6842 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-7.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-7.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: null is not of type "object" + 0: input does not match expected validation schema + 1: null is not of type "object" diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-8.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-8.snap index db1cb0e7..9ba0fb58 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-8.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-8.snap @@ -3,12 +3,11 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Action +failed to load file://@@INPUT@@ as action Caused by: - 0: failed to load action from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: runs: Additional properties are not allowed ('image' was unexpected) + 0: input does not match expected validation schema + 1: runs: Additional properties are not allowed ('image' was unexpected) runs: "using" is a required property runs: "main" is a required property runs: Additional properties are not allowed ('image' was unexpected) diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-9.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-9.snap index 64127fce..ff6e11db 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-9.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs-9.snap @@ -3,9 +3,8 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Action +failed to load file://@@INPUT@@ as action Caused by: - 0: failed to load action from file://@@INPUT@@ - 1: invalid YAML syntax: mapping values are not allowed in this context at line 3 column 8 - 2: mapping values are not allowed in this context at line 3 column 8 + 0: invalid YAML syntax: mapping values are not allowed in this context at line 3 column 8 + 1: mapping values are not allowed in this context at line 3 column 8 diff --git a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs.snap b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs.snap index aa3e8c3b..80b49a99 100644 --- a/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs.snap +++ b/crates/zizmor/tests/integration/snapshots/integration__e2e__invalid_inputs.snap @@ -3,11 +3,10 @@ source: crates/zizmor/tests/integration/e2e.rs expression: "zizmor().expects_failure(true).input(input_under_test(&format!(\"invalid/{workflow_tc}.yml\"))).args([\"--strict-collection\"]).run()?" --- fatal: no audit was performed -failed to load input as Workflow +failed to load file://@@INPUT@@ as workflow Caused by: - 0: failed to load workflow from file://@@INPUT@@ - 1: input does not match expected validation schema - 2: jobs.invalid: "runs-on" is a required property + 0: input does not match expected validation schema + 1: jobs.invalid: "runs-on" is a required property jobs.invalid: Additional properties are not allowed ('steps' was unexpected) jobs.invalid: "uses" is a required property