Three issues from PR review, all straightforward:
The cron validator was rejecting day-of-week value 7, which is a
perfectly valid Sunday alias in both POSIX cron and GitHub Actions.
The max was 6 when it should be 7. The named-value resolver guard
also needed updating from `max == 6` to `max >= 6` so named days
still resolve correctly with the wider range.
The `evaluate_job_condition` heuristic for detecting `steps.*`
references was using a bare `contains("steps.")`, which means an
env var like `env.MY_STEPS_COUNT` would falsely trigger it and
short-circuit to false. Now we check that the character before
"steps." is either start-of-string or non-alphanumeric. Not a
full expression parser, but it stops the obvious false positives.
While at it, add a `Step::with_run` constructor so the GitLab
converter doesn't need three identical 12-field struct literals
that silently break every time someone adds a field to Step.
The step-level if-condition check and continue-on-error handling was
copy-pasted between execute_job and execute_matrix_job with subtly
different control flow — one sets job_success=false and breaks, the
other returns Ok(JobResult{Failure}) immediately. Two copies of the
same logic that *already* disagree is not redundancy, it's a bug
waiting to happen. Let's fix that.
Extract run_step_with_guards() that encapsulates the if-condition
evaluation, execute_step call, and continue-on-error wrapping into
a single StepOutcome enum. Both job execution paths now call this
shared helper.
While at it, fix the condition evaluator defaulting bare steps.*
references to true — "steps.build.outcome == 'failure'" should
*not* optimistically run the step. Now only always() and success()
default to true; everything else (bare step refs, failure(),
cancelled()) conservatively defaults to false.
Also add serde alias "matrix" on Job.strategy so old workflows with
flat matrix: at job level still parse, and document the intentional
or_insert_with in matrix include merging per GitHub Actions spec.
The PR review turned up a few things that needed fixing before this
was actually ready.
The step-level `if` condition evaluator was silently reusing the
job-level `evaluate_job_condition` function, which knows nothing
about step-scoped expressions like `steps.<id>.outcome`, `success()`,
`failure()`, `always()`, or `cancelled()`. These would fall through
to the generic "unknown condition" path without so much as a warning.
Now they're detected early, a warning is logged, and they default to
true — which is at least *honest* about the limitation.
The volume path traversal check (`..`) was applied to the entire
volume spec string, meaning a perfectly legitimate container path
like `/safe/host:/container/..weird` would get rejected. The check
now only inspects the host path component after splitting on `:`,
which is the part that actually matters for traversal attacks.
While at it, renamed the awkwardly-named `step_name_for_skip` to
just `step_name` in `execute_matrix_job` for consistency with
`execute_job`, and added a BREAKING_CHANGES.md documenting the
EncryptedSecretStore serialization format change.
Added 19 new tests covering matrix include/exclude merge semantics,
step condition evaluation for unsupported expressions, volume path
traversal edge cases, and continue-on-error + step-level if parsing.
A thorough manual verification of every feature uncovered a
*remarkable* collection of bugs hiding in plain sight. The
highlights:
The `strategy.matrix` YAML structure was never parsed. The Job
struct had `matrix` at the top level, but GitHub Actions nests it
under `strategy.matrix`. Serde silently ignored the `strategy`
key, so matrix expansion code existed but could never run. For
absolutely no reason. Introduce a proper `Strategy` struct and
wire it through the executor.
The Step struct was missing `if`, `id`, `working-directory`,
`shell`, and `timeout-minutes` fields. Step-level conditionals
were silently dropped — every step always ran regardless of its
`if` condition. While at it, `continue-on-error` was in the
struct but had no serde rename and was never checked during
execution. Fix all of that.
The validator cheerfully reported cyclic `needs` dependencies as
"Valid". Add DFS cycle detection so `A -> B -> C -> A` is caught
at validation time instead of blowing up at execution time.
Five of eight GitLab CI test fixtures failed to parse because the
model was too rigid: `extends` only accepted arrays (not strings),
`variables` rejected integers, `cache.key` rejected structured
formats, and `script` rejected single strings. Add custom
deserializers following the existing codebase pattern.
The GitHub trigger function leaked the auth token via curl process
arguments visible in `/proc/[pid]/cmdline`. Replace with reqwest,
matching the pattern already used elsewhere. Also add symlink and
path traversal protections in the executor.
Other fixes: hardcoded matrix variable stripping replaced with
proper substitution, `show_action_messages` wired through TUI,
dead `if true {}` removed, default branch detection uses remote
HEAD instead of current branch, cron validator accepts named
days/months, reusable workflow ref validation loosened from OR
to AND, matrix include entries merge into all matching combos.
It turns out that if someone writes `container:` with an empty image
string, we'd happily pass "" to Docker and let it figure out what
that means. Spoiler: it doesn't.
Similarly, volume specs like "/host:" or ":/container" would produce
a PathBuf::from("") mount, which is the kind of thing that makes
container runtimes *very* unhappy. Let's just skip those with a
warning instead of pretending they're valid.
While at it, replace the derived Serialize on ContainerCredentials
with a custom impl that redacts the password field. The Debug impl
was already doing this, but serde_json::to_string was still happily
dumping passwords in plaintext. Please don't do that.
ContainerCredentials had a derived Debug impl that would happily
dump passwords into logs, panic output, and anywhere else Debug
gets called. That's *exactly* the kind of thing that bites you at
3am when someone adds a debug trace and suddenly credentials show
up in plaintext in your log aggregator.
Replace the derived Debug with a manual impl that redacts the
password field. While at it, add a guard for empty volume specs
that would otherwise produce undefined Docker behavior, a note
about the splitn limitation with Windows paths, and fix clippy
warnings on the test assertions.
The container directive support in 2eae320 had ~45 lines of identical
volume-mounting and env-path-remapping code copy-pasted between the
Docker-action execution branch and the run-step branch. That's not
redundancy, that's a future bug waiting to happen in two places
instead of one.
Extract `prepare_container_mounts()` to handle the shared logic:
GitHub env file remapping, container-defined volume parsing, and the
runtime mode detection. Both branches now call into the same function.
While at it, fix single-path volume specs (e.g. "/data" without a
colon) which were being silently dropped because the code only
handled the `host:container` format. Now they mount at the same path
inside the container, which matches GitHub Actions behavior.
Also add `warn_unsupported_container_fields()` so users actually
*know* when their `options`, `credentials`, or `ports` fields are
being ignored rather than discovering it the hard way in production.
Add parser tests for `deserialize_container` covering string format,
full object format, absent container, and registry image with colon
in the tag.
It turns out that the Job struct in the parser had *no* container
field at all. When a workflow specified `container: alpine:3.22.1`,
serde silently dropped it, and the engine happily derived the runner
image from `runs-on` instead. So `apk add` runs inside Ubuntu.
Confusion ensues.
Add a JobContainer type with a custom deserializer that handles both
the string form (`container: alpine:3.22.1`) and the object form
(`container: { image: ..., env: ..., volumes: ... }`). A new
get_effective_runner_image() prefers the container image over the
runs-on mapping.
While at it, fix the GITHUB_ENV volume mounting for real container
runtimes. The old code identity-mounted the host temp path into the
container, which breaks on macOS with Podman because /var/folders
doesn't exist in the VM. Now we mount the github env directory at
/github/workflow/ and remap the env vars to match.
Container-level env vars and volumes are also wired through with
correct precedence (step > job > container).
Closes#58
It turns out that action references like `github/codeql-action/init@v3`
were being treated as if `github/codeql-action/init` was the repo name.
The resolver would then try to fetch action.yml from
`github/codeql-action/init/v3/action.yml` instead of the correct
`github/codeql-action/v3/init/action.yml`. Same bug hit shallow_clone
— it would try to clone a repo URL with the sub-path baked in, which
obviously doesn't exist.
Add a `sub_path` field to `ActionInfo` so `resolve_action` splits
`owner/repo/path@ref` into its actual components. The resolver,
cache key, and composite action clone all use the sub-path correctly
now.
While at it, stop using `std::env::set_var`/`remove_var` in the
wiremock tests. Those are unsound in multi-threaded test binaries
(Rust 1.83+ rightly marks them unsafe). Refactored `fetch_and_parse`
to accept the token as a parameter — the tests just pass it directly,
no env mutation needed.
Three issues from code review, all small but all real:
The echo fallback in execute_step was interpolating the `uses` string
directly into a single-quoted sh -c argument. A workflow with a
single quote in the action ref would break out of the shell string.
Escape single quotes with the standard '\'' pattern.
The fetch_and_parse tests were calling env::remove_var("GITHUB_TOKEN")
and env::set_var() without saving and restoring the original value.
If GITHUB_TOKEN was set before the test suite ran, it would be
permanently wiped for subsequent tests. All three tests now
save/restore properly.
While at it, document the ActionInfo::version field semantics —
it's empty for docker/local refs, holds the git ref for GitHub
action refs, and defaults to "main" when omitted. Future readers
shouldn't have to guess.
It turns out that resolve_action was blindly splitting on '@' for
*all* action references, including Docker image refs like
docker://alpine@sha256:abc123. The '@' in a Docker digest is not a
version separator — it's part of the image reference. Splitting it
produces a nonsensical repository and a fake "version" that happens
to be a SHA256 digest. Nobody noticed because the Docker path
doesn't use the version field, but the parsed data was still wrong.
While at it, the auth retry path in fetch_and_parse was constructing
a brand new reqwest::Client on every single 404-then-retry cycle.
That means a fresh TLS handshake each time, which is wasteful when
we already have a perfectly good static client pattern. Promote the
no-redirect client to a static Lazy, same as HTTP_CLIENT.
The auth redirect flow — where we send GITHUB_TOKEN to the origin
but strip it before following a redirect to a CDN — had zero test
coverage. This is the kind of security invariant that *really*
should not depend on code review alone. Add wiremock-based tests
that verify the token does not leak to redirect targets, plus tests
for the basic auth retry and 404 paths. Parameterize fetch_and_parse
with a base_url so wiremock can intercept the requests.
Three issues from code review, all minor but all worth fixing before
they confuse someone later.
The BoundedCache was documented as "LRU-style" when it's actually
plain FIFO — get() doesn't promote keys. Nobody cares for this use
case since actions resolve once per run, but calling FIFO "LRU" is
the kind of lie that breeds real bugs when someone trusts the docs
and adds access-pattern-dependent logic later. Fixed the comments.
The 404-retry-with-GITHUB_TOKEN pattern in fetch_and_parse was
correct but undocumented — it *only* targets raw.githubusercontent.com
so there's no token-to-attacker-host risk, but that's the kind of
thing you want a future reader to see immediately without having to
trace the URL construction. Added a comment.
resolve_action was setting version to "main" for docker:// refs and
local paths (./), which is semantically wrong. Docker refs embed
their tag in the repository string, and local paths have no version
at all. Neither value was ever *used* in those code paths, but a
wrong value sitting in a struct field is a bug waiting to happen.
Set version to "" for both cases instead.
The previous commit added remote action.yml resolution, which was a
good idea in principle. But it had a *rather significant* problem:
when an action declares `runs.image: Dockerfile` (meaning "build my
bundled Dockerfile"), the resolver happily returned the literal
string "Dockerfile" as the Docker image name.
Confusion ensues. Downstream code tries to pull an image called
"Dockerfile" from a registry. That doesn't work.
Add a DockerBuild variant to ActionType for actions that bundle
their own Dockerfile. image_for_action() now returns Option<String>
— None for DockerBuild — so the caller falls back to the hardcoded
mapping instead of trying to pull nonsense from a registry.
While at it, fix several other problems from the initial PR:
- Reuse a static reqwest::Client instead of creating one per HTTP
request, because TLS initialization on every fetch is wasteful
- Capture git clone stderr instead of sending it to /dev/null, so
when cloning a remote composite action fails you actually get to
know *why*
- Add tests for ActionInfo.version parsing in the parser (the field
was added but never tested — please don't do that)
- Add edge-case tests for DockerBuild, unknown using values, missing
fields, and the docker://Dockerfile prefix variant
Fixes#48. When encountering third-party GitHub Actions, wrkflw previously
defaulted to node:20-slim for all unknown actions. Now it fetches the
action's action.yml from raw.githubusercontent.com, parses runs.using to
determine the action type (Node/Docker/Composite), and selects the
appropriate Docker image. Falls back to the existing hardcoded mapping on
any failure.
- Add custom deserializer for runs-on field to handle both string and array formats
- Update Job struct to use Vec<String> instead of String for runs-on field
- Modify executor to extract first element from runs-on array for runner selection
- Add test workflow to verify both string and array formats work correctly
- Maintain backwards compatibility with existing string-based workflows
Fixes issue where workflows with runs-on: [self-hosted, ubuntu, small] format
would fail with 'invalid type: sequence, expected a string' error.
This change aligns with GitHub Actions specification which supports:
- String format: runs-on: ubuntu-latest
- Array format: runs-on: [self-hosted, ubuntu, small]
- Copied schema files into parser crate src directory
- Updated include_str! paths to be relative to source files
- Ensures schemas are bundled with crate during publish
- Resolves packaging and verification issues during publication
Fixes the build error that was preventing crate publication.
- Updated include_str! paths from ../../../ to ../../../../
- This resolves packaging issues during cargo publish
- Fixes schema loading for parser crate publication
- Add support for job-level if conditions with basic expression evaluation
- Support both string and array formats for job needs field (needs: job vs needs: [job])
- Add missing job fields: if_condition, outputs, permissions to Job struct
- Implement job condition evaluation in executor with pattern matching for:
- Simple boolean conditions (true/false)
- GitHub event conditions (github.event.pull_request.draft == false)
- Job output conditions (needs.jobname.outputs.outputname == 'value')
- Jobs with false conditions are now properly skipped with appropriate logging
- Fixes parsing issues with workflows that use changes jobs and conditional execution
Resolves compatibility with workflows like iceoryx2 that use path filtering patterns.
This commit adds full support for GitLab CI/CD pipelines:
- Add GitLab CI pipeline models with complete spec support (jobs, stages, artifacts, cache, etc.)
- Implement GitLab CI/CD pipeline parsing and validation
- Add schema validation against GitLab CI JSON schema
- Support automatic pipeline type detection based on filename and content
- Add GitLab-specific CLI commands and flags
- Implement pipeline conversion for executor compatibility
- Add validation for common GitLab CI configuration issues
- Update CLI help text to reflect GitLab CI/CD support
- Support listing both GitHub and GitLab pipeline files
This expands wrkflw to be a multi-CI tool that can validate and execute both GitHub
Actions workflows and GitLab CI/CD pipelines locally.
- Extracted functionality from the `src/` directory into individual crates within the `crates/` directory. This improves modularity, organization, and separation of concerns.
- Migrated modules include: models, evaluator, ui, gitlab, utils, logging, github, matrix, executor, runtime, parser, and validators.
- Removed the original source files and directories from `src/` after successful migration.
- This change sets the stage for better code management and potentially independent development/versioning of workspace members.
