-
Notifications
You must be signed in to change notification settings - Fork 315
Description
Summary
In actions/setup/js/check_workflow_timestamp_api.cjs, the lock newer branch silently passes when the .lock.yml commit is newer than the source .md but the embedded frontmatter hash does not match the recomputed hash. The check logs ⚠️ Frontmatter hash mismatch detected and then ✅ Lock file is up to date without calling core.setFailed. By contrast, the workflow newer branch correctly calls core.setFailed on hash mismatch. This asymmetry means a tampered lock file that has been committed or rebased to have a newer timestamp than the source .md will pass the integrity guard and proceed to execution.
The rationale in the source comment — "The .md file may have been edited after the lock was compiled" — is circular: if .md was edited after the lock, the .md would be newer, not the lock. A lock that is newer with a mismatched hash is the signature of post-compilation tampering.
Affected Area
Compile-time / activation-time lock integrity boundary — actions/setup/js/check_workflow_timestamp_api.cjs, lock newer branch (hash mismatch path).
Reproduction Outline
- Create a valid workflow pair:
workflow.mdand its compiledworkflow.lock.yml. - Tamper
workflow.lock.yml— e.g., add a broader trigger, elevate permissions, or inject a privileged job — without modifying the source.md. - Commit and push so that the tampered lock file's commit timestamp is newer than the source
.mdcommit (trivially achieved by amending or rebasing). - Trigger workflow activation; the runtime executes
check_workflow_timestamp_api.cjs.
Observed Behavior
The check logs a frontmatter hash mismatch warning but does not call core.setFailed. Activation continues as if the lock is valid and up to date. The test suite explicitly asserts this permissive behavior (it("should pass when lock file is newer but hashes differ", ...)).
Expected Behavior
Any frontmatter hash mismatch — regardless of which file has the newer timestamp — should cause core.setFailed to be called, blocking execution. If there is a legitimate permissive case (e.g., compiler schema migration), it should be gated behind an explicit opt-in flag and documented, not treated as the default.
Security Relevance
An attacker with write access (or the ability to get a tampered commit merged via PR review gap or compromised automation) can modify a .lock.yml's triggers, permissions, or job definitions and bypass the hash integrity guard entirely by ensuring the lock commit is newer than the source. This undermines the core security guarantee of the activation-time freshness/integrity check.
Suggested Fix
- In
check_workflow_timestamp_api.cjs, callcore.setFailedon hash mismatch in thelock newerbranch, making it symmetric with theworkflow newerbranch. - Update tests to assert
setFailedis invoked on hash mismatch in thelock newercase. - If a permissive case is intentional, document it explicitly and require an opt-in flag.
gh-aw version: v0.64.2
Original finding: https://github.com/githubnext/gh-aw-security/issues/1549
Generated by File Issue · ◷