fix(ci): zip-slip-safe extraction in sync-skills workflow#70
Open
phjlljp wants to merge 1 commit into
Open
Conversation
Plain `unzip -o` does not validate archive entry names. A malicious
release zip in PostHog/posthog or PostHog/context-mill — or any
upstream typosquat — could carry `../foo` or `/abs/foo` entries that
write outside `skills/` during the daily sync, tampering with workflow
files, hooks, or other ai-plugin source. Replace the three `unzip -o`
calls with a `safe_unzip` helper that uses python's `zipfile` and
rejects any entry whose resolved path would escape the destination.
Also adds `permissions: {}` at workflow root. The actual writes flow
through the explicit GH App token already plumbed in, so the implicit
`GITHUB_TOKEN` does not need any scopes — default-deny is the right
posture.
This PR intentionally does not touch the auto-merge step at the end of
the workflow. That is a maintainer policy decision and out of scope for
the security fix.
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.
Summary
unzip -ocalls insidesync-skills.ymlwith asafe_unzipshell function backed by python'szipfilemodule. Every entry's resolved path is checked against the destination root; absolute paths and..traversal are rejected before any member is extracted.permissions: {}at the workflow root. The destructive writes (commit, PR creation, auto-merge) flow through the explicit GH App token already plumbed into the workflow, so the implicitGITHUB_TOKENdoes not need any scope.Why
unzip -ohappily writes to whatever path the archive specifies — including../fooand/etc/foo— meaning a tamperedskills.zipfromPostHog/posthogorskills-mcp-resources.zipfromPostHog/context-millcan write outsideskills/during the daily sync. The reachable blast radius on the runner includes.github/workflows/,hooks/,scripts/, and.git/— all files the next workflow run picks up viaactions/checkout. Even though the immediate trust chain (PostHog/posthog → ai-plugin) is internal, the workflow runs daily on a cron with broad write authority, so it deserves defensive parsing rather than implicit trust.The auto-merge step at the end of the workflow is intentionally untouched — that is a maintainer policy decision and out of scope for the security fix.
Test plan
python3 -c "import yaml; yaml.safe_load(open('.github/workflows/sync-skills.yml'))"parses cleanlyclean.zipwith normal entries → extracts (rc=0)traverse.zipcontaining../escaped.txt→ refused (rc=1, "traversal or absolute path")absolute.zipcontaining/etc/test.txt→ refused (rc=1, "traversal or absolute path")agent-skills-latestand context-mill releasesNotes
This is one of three security PRs from a strict-zero-trust audit; the other two cover
POSTHOG_HOSTvalidation (#68) and thegate-exec-write.shparser bypasses (#69). They land independently of this one.