v0.2: FindingSurface typing + shell/Dockerfile detectors + workflow-permissions hardening#15
Merged
Merged
Conversation
…rface field
Adds the typing and dispatch infrastructure the v0.2 detectors expansion
builds on, with no behavior change to existing detectors.
types.ts: introduces FindingSurface ('source' | 'package' | 'workflow' |
'container'), adds it as a required field on Finding so reviewers can
filter by surface, and extends DiffContext with newFileContents (full
file contents keyed by path) and scannedSurfaces (which surfaces the
diff actually covered).
paths.ts: replaces the boolean isScannable() with surfaceForPath(),
which returns the FindingSurface for each file. isScannable() is kept
as a thin wrapper for back-compat. Recognizes .sh/.bash/.zsh/.ps1/.psm1
as shell and Dockerfile* as container surfaces; these have no detector
yet (lands in the next commit) but the dispatch is ready.
git-diff.ts: collects newFileContents during the diff so detectors that
need cross-line context (multi-line install scripts, Dockerfile HEREDOCs,
etc.) can pull it without re-reading from disk. Also tracks which
surfaces were actually present in the diff.
report.ts: threads FindingSurface through the markdown/json/github output
so reviewers see findings grouped by surface.
Existing detectors (js, py, package-scripts, package-deps, workflow-permissions)
are minimally updated to populate the new required 'surface' field on
their finding outputs. No detection logic changes.
Two new detectors slotted into the surface-aware dispatch added by the previous commit. Both are pure additions; nothing existing changes. shell-capability.ts (surface: 'source') scans added lines in shell scripts (.sh, .bash, .zsh, .ps1, .psm1) for pipe-to-shell installers, remote curl/wget pipelines, and PowerShell DownloadString/iex chains — the same payload shapes that already trip the js/py detectors when the agent generates code, but in the script bodies the agent generates next to that code. dockerfile-capability.ts (surface: 'container') scans added lines in Dockerfile* (including platform/arch-suffixed variants like Dockerfile.alpine) for the same install-time-execution shapes plus container-specific shapes: ADD https://… (silent network fetch into the image), RUN curl … | sh (the docker-equivalent of postinstall), chmod 777 and USER root weakenings. Both ship with their own fixture-driven tests under test/*.test.mjs. diff.ts wires them into the pipeline alongside the existing detectors. No change to existing detector signatures or behavior.
…ence output
Substantial expansion of workflow-permissions to cover what real-world
agent-generated workflow YAML actually does, plus the matching Action
runtime entry and test surface.
Workflow detector (src/detectors/workflow-permissions.ts, +250 LOC):
- pull_request_target trigger detection with cross-line awareness
via newFileContents (matches PR-head checkout under elevated
privilege)
- Self-hosted runner detection (PR-triggered runners on private
infrastructure)
- Mutable action reference detection (unpinned @main / @latest /
@v* / @Branch — anything not a pinned SHA)
- Docker host control detection (--privileged, socket mounts,
--device, --cap-add)
- Secret env var tracking across the file: env-secret references
are now matched against external requests anywhere in the same
workflow, not just the same line
- GitHub-token write permission detection broadened to the full
set of token permission keys (actions, attestations, checks,
contents, deployments, discussions, id-token, issues, packages,
pages, pull-requests, security-events, statuses)
Action runtime (src/action.ts + action.yml):
- New adoption-evidence output (redacted JSON for sharing in team
feedback, no file paths or raw finding content)
- Top-recommendations output for downstream PR-comment workflows
- Hardened fail-on input normalization with friendly error on bad
threshold values
- Missing-git-ref error path emits a structured message instead
of a stack trace
diff.ts wires newFileContents through to workflow-permissions, js,
and py — the cross-line context their hardened forms need.
Test surface:
- workflow.test.mjs: +329 LOC covering every new detector branch
- detectors.test.mjs: +280 LOC integration coverage
- git-diff.test.mjs: +516 LOC for the newFileContents collection
and pull_request_target file scanning
- cli-output.test.mjs: new outputs covered
- py-capability.test.mjs: env-var secret exfil case
Total: 64/64 tests passing.
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
Rescues ~2,500 lines of orphaned codex WIP into three reviewable commits. Backup branch with the full original tree is preserved at
rescue/orphaned-codex-wip-backup.The work splits into three logically-independent commits. Each builds and tests cleanly on its predecessor. Review commit-by-commit for the best experience.
1.
Foundation: FindingSurface typing + surface dispatchtypes.ts:FindingSurface = 'source' | 'package' | 'workflow' | 'container', added as required field onFinding.DiffContextextended withnewFileContentsandscannedSurfaces.paths.ts: newsurfaceForPath()dispatch returning the right surface per file. Recognizes.sh/.bash/.zsh/.ps1/.psm1as shell andDockerfile*as container.git-diff.ts: collects new-side file contents during diff so detectors that need cross-line context don't re-read disk.report.ts: threads surface through markdown / JSON / GitHub annotation output.surfacefield. No detection logic changes.2.
Add shell and Dockerfile capability detectorssrc/detectors/shell-capability.ts(surface:source) — pipe-to-shell, remote curl/wget pipelines, PowerShelliex (irm ...)chains in.sh/.bash/.zsh/.ps1/.psm1.src/detectors/dockerfile-capability.ts(surface:container) — same payload shapes plus container-specific signals:ADD https://…,RUN curl … | sh,chmod 777,USER root.diff.tswires them in.3.
Workflow-permissions hardening + Action runtime entry + adoption-evidence outputSubstantial workflow-permissions expansion. New detectors:
pull_request_targettrigger detection with cross-line awareness for PR-head checkout under elevated privilege--privileged, socket mounts,--device,--cap-add)Action runtime hardening:
adoption-evidenceAction output (redacted JSON, no file paths or raw finding content) for sharing in team feedbacktop-recommendationsoutput for PR-comment workflowsfail-oninput normalized with friendly error on bad thresholdVerification
npm run build— cleannpm test— 64/64 passingNotes
A handful of files from the original WIP were deliberately left out: feedback templates and team-validation docs that contained commercial-tier framing inconsistent with the public README. Those remain available on
rescue/orphaned-codex-wip-backupfor separate review.Test plan