Skip to content

Lock freshness check must fail closed on frontmatter hash mismatch regardless of lock/source timestamp ordering #23559

@szabta89

Description

@szabta89

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

  1. Create a valid workflow pair: workflow.md and its compiled workflow.lock.yml.
  2. Tamper workflow.lock.yml — e.g., add a broader trigger, elevate permissions, or inject a privileged job — without modifying the source .md.
  3. Commit and push so that the tampered lock file's commit timestamp is newer than the source .md commit (trivially achieved by amending or rebasing).
  4. 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

  1. In check_workflow_timestamp_api.cjs, call core.setFailed on hash mismatch in the lock newer branch, making it symmetric with the workflow newer branch.
  2. Update tests to assert setFailed is invoked on hash mismatch in the lock newer case.
  3. 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 ·

Metadata

Metadata

Labels

No labels
No labels

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions