Refine handling of permissions between reusable caller/called workflows

[notdodo]

Pre-submission checks

• I am not filing a feature request. These should be filed via the feature request form instead.
• I have looked through the open issues for a duplicate report.

Expected behavior

zizmor shouldn't notify "overly broad permissions" if the permissions are set on the reusable workflow steps.

Actual behavior

When a reusable workflow, with permissions set, is used in an actual workflow zizmor reports:

Reproduction steps

Reusable workflow: https://github.com/notdodo/github-actions/blob/main/.github/workflows/clean-branch-cache.yml
The permissions are set at job level

jobs:
  cleanup:
    name: Cleanup caches for a merged branch
    runs-on: ${{ inputs.runs-on }}
    permissions:
      actions: write

Running workflow: https://github.com/notdodo/github-actions/blob/main/.github/workflows/repo-branch-cleanup.yml
The workflow just uses the reusable workflow.

Logs

Additional context

Zizmor workflow: https://github.com/notdodo/github-actions/blob/main/.github/workflows/infra-security-scan.yml#L71

[woodruffw]

Thanks for the report @notdodo. To make sure I understand: you're reporting that zizmor gives you a finding on the reusable workflow definition itself, despite the call to that reusable workflow having its permissions constrained?

If that summary is correct, then the current behavior is intentional: zizmor doesn't know how many times a reusable workflow is called or whether it's called outside of the repository being analyzed, so we conservatively assume that the workflow definition itself also needs to constrain the permissions.

Let me know if I got the above right, and if the subsequent expiration makes sense! I'm a little bit murky on how reusable workflow permissions intersect between caller/callee, so it's also possible the definition's own permissions don't matter when the only trigger is a reusable workflow call.

[notdodo]

Thank you @woodruffw for the quick response.

The issue is reported on the caller workflow, in this case repo-branch-cleanup.yml, that calls the reusable clean-branch-cache.yml where the permissions are defined.

so we conservatively assume that the workflow definition itself also needs to constrain the permissions.
I'm a little bit murky on how reusable workflow permissions intersect between caller/callee, so it's also possible the definition's own permissions don't matter when the only trigger is a reusable workflow call.

I think that the two sentences above cleared me up as I don't know if a caller can override/extend callee permissions and, if needed for this issue, I can return to you with an answer.
I just assumed that this wasn't possible.

[woodruffw]

No problem!

Yeah, if you could test how GHA behaves here, I would greatly appreciate it! But of course no problem if not; I'll get around to it eventually.

(I think you're right that the caller can't expand the scope defined in the workflow itself, but the workflow should also define a minimal set so that the caller can't pass overscoped credentials in by accident. But that's speculation since I don't know how it actually behaves.)

[notdodo]

Hey @woodruffw; it seems that the caller can't override the reusable workflow permissions.
See result here: https://github.com/notdodo/test-GHAs/actions/runs/13358053020
Reusable: https://github.com/notdodo/test-GHA2/blob/main/.github/workflows/reusable.yml
Caller: https://github.com/notdodo/test-GHAs/blob/main/.github/workflows/caller.yml

And if I remove the actions: read from the caller I get The workflow is not valid. .github/workflows/caller.yml (Line: 23, Col: 3): Error calling workflow 'notdodo/test-GHA2/.github/workflows/reusable.yml@main'. The nested job 'cleanup' is requesting 'actions: write', but is only allowed 'actions: none'.

And obviously if I remove the permissions setting on the reusable I can set whatever I want on the caller.

[woodruffw]

Thanks for trying those @notdodo!

Based on that, it sounds like the following holds:

1. The reusable workflow (i.e. called workflow) sets its own permissions, which the caller cannot expand.
2. The caller can further restrict the called permissions by explicitly specifying them.

Given that, I think the current behavior is intentional: zizmor doesn't currently resolve the called workflow's own permissions, since it might be remote. As such, the audit assumes that the caller needs to explicitly constrain the permissions as well.

Long term we could probably make this better by doing more "interprocedural"-type analysis within zizmor, i.e. actually resolving the reusable workflow and seeing what it actually does vis a vis permissions.