Skip to content

Fix zizmor 1.29 finding path parsing - #58339

Merged
pelikhan merged 1 commit into
mainfrom
copilot/investigate-zizmor-failure
Sep 4, 2026
Merged

Fix zizmor 1.29 finding path parsing#58339
pelikhan merged 1 commit into
mainfrom
copilot/investigate-zizmor-failure

Conversation

Copilot AI commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Zizmor 1.29 findings were discarded because its JSON output uses verbatim_path instead of given_path. Strict-mode compilation consequently failed with the misleading message zizmor found 0 security warnings/errors.

Changes

  • Current schema support

    • Parse locations[].symbolic.key.Local.verbatim_path.
    • Preserve compatibility with legacy given_path output.
  • Regression coverage

    • Cover zizmor 1.29 output and verify findings retain their file location, severity, and message.

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot AI requested a review from pelikhan September 4, 2026 00:44
@pelikhan
pelikhan marked this pull request as ready for review September 4, 2026 00:47
Copilot AI balanced review requested due to automatic review settings September 4, 2026 00:47
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Ponytail Reviewer failed. Please review the logs for details.

Warning

Threat Detection Engine Failure — The analysis engine could not complete. This is a tooling failure, not a security finding.

What happened

The threat detection engine failed to produce results.

Review the workflow run logs for details.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • ab.chatgpt.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "ab.chatgpt.com"

See Network Configuration for more information.

Generated by Ponytail Reviewer for #58339

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

PR Code Quality Reviewer completed the code quality review.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Test Quality Sentinel completed test quality analysis.

Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff

🧪 Test quality analysis by Test Quality Sentinel

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.

No ADR enforcement needed: PR does not have the 'implementation' label and has ≤100 new lines of code in business logic directories.

🏗️ ADR gate enforced by Design Decision Gate 🏗️

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Skills-Based Review 🧠

Applied /diagnosing-bugs — approving, no actionable issues found.

📋 Key Themes & Highlights

Root cause analysis

  • Correctly identifies the actual root cause: zizmor 1.29 renamed given_path to verbatim_path in its JSON schema, not a transient parsing bug.
  • Fix falls back to verbatim_path only when given_path is empty, preserving backward compatibility with older zizmor output — good defensive sequencing.

Regression coverage

  • New test case (current zizmor output uses verbatim path) uses realistic zizmor 1.29 JSON shape and asserts the full formatted output line (path, line, column, severity, message), which would have caught this exact regression.
  • Existing given_path-based test cases remain untouched, confirming legacy compatibility isn't broken.

Positive Highlights

  • ✅ Minimal, surgical diff (5 lines in the fix, 38 in tests) — no unrelated changes.
  • ✅ Single usage site was updated consistently; no other GivenPath references were missed.

No blocking issues. Small, well-tested fix.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 18 AIC · ⌖ 14.5 AIC · ⊞ 10.3K
Comment /matt to run again

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Comment Memory

reviewed_at: 2026-09-04T00:50:38Z
review_event: COMMENT
top_themes:
  - no blocking issues found in zizmor path fallback
files_reviewed:
  - pkg/cli/zizmor.go
  - pkg/cli/zizmor_test.go
comment_count: 0

Note

This comment is managed by comment memory.

It stores persistent context for this thread in the code block at the top of this comment.
Edit only the text inside the backtick fences; workflow metadata and the footer are regenerated automatically.

Learn more about comment memory

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 22.8 AIC · ⌖ 8.57 AIC · ⊞ 21.8K ·
Comment /review to run again

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Verdict

No blocking issues in this patch.

What I checked

The verbatim_path fallback is scoped to the same JSON field family as the legacy given_path, and the added regression test covers the current zizmor 1.29 payload shape that was dropping findings. I did not find a changed-line correctness or maintainability problem worth blocking this PR on.

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 22.8 AIC · ⌖ 8.57 AIC · ⊞ 21.8K
Comment /review to run again

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

All reviewed changes are covered by regression tests with no unresolved issues.

Pull request overview

Updates zizmor JSON path parsing for v1.29 while preserving legacy compatibility.

Changes:

  • Supports verbatim_path with a given_path fallback.
  • Adds regression coverage for finding details.
File summaries
File Description
pkg/cli/zizmor.go Parses current and legacy path fields.
pkg/cli/zizmor_test.go Tests zizmor v1.29 output handling.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 0
  • Review effort level: Balanced

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Great work! 🎉 This fix for zizmor 1.29 finding path parsing looks solid. The PR correctly addresses the JSON output schema change (verbatim_path vs given_path) and maintains backward compatibility with legacy output. The regression test coverage ensures this won't break in the future. This is ready for review!

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • github.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "github.com"

See Network Configuration for more information.

Generated by ✅ Contribution Check · copilot · auto · 53.2 AIC · ⌖ 6.63 AIC · ⊞ 9.3K ·

@pelikhan
pelikhan merged commit 757e3a3 into main Sep 4, 2026
103 of 115 checks passed
@pelikhan
pelikhan deleted the copilot/investigate-zizmor-failure branch September 4, 2026 02:16
@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🎉 This pull request is included in a new release.

Release: v0.88.4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants