Allow local actions outside the workspace

[jenseng]

Fixes #2107

Also simplify actionName logic and ensure it returns sensible values for actions outside the workspace (it's only used for logging and docker image name)

[codecov]

Codecov Report

Attention: Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Project coverage is 74.67%. Comparing base (5a80a04) to head (40af543).
Report is 201 commits behind head on master.

Files with missing lines	Patch %	Lines
pkg/runner/action.go	55.55%	3 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #2108       +/-   ##
===========================================
+ Coverage   61.56%   74.67%   +13.11%     
===========================================
  Files          53       73       +20     
  Lines        9002    11132     +2130     
===========================================
+ Hits         5542     8313     +2771     
+ Misses       3020     2183      -837     
- Partials      440      636      +196     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ChristopherHX]

In my opinion is Allow local actions outside the workspace more like an exploit (regarding security) for action/runner than an actual feature.

It doesn't look to be far away from Issue 2: Arbitrary file download in artifact server (GHSL-2023-004) of GHSA-pc99-qmg4-rcff.

[jenseng]

In my opinion is Allow local actions outside the workspace more like an exploit (regarding security) for action/runner than an actual feature.

For this change we're talking about reading an existing action.yml file from another directory, not reading or writing arbitrary files. So while there are superficial similarities to GHSL-2023-004, a better comparison would be run: ../../whatever.sh, which is not restricted by act or actions/runner (nor would it be feasible to do so IMO).

In the absence of being able to use contexts within uses, being able to use such a path is useful when working with composite actions in a monorepo, as explained on Rationale section of #2107

[ChristopherHX]

Yes we both have different opinions here... the good thing for you is that my opinion alone doesn't prevent merging this.

Keep in mind doing this in monorepos has an old unfixed bug when referencing local actions in actions/runner (those action which has post steps, like actions/cache, actions/checkout): actions/runner#2009, input and outputs end up with wrong values...

• If GitHub Actions would allow absolute paths in uses: /path/to/action, then it wouldn't be an exploit for me.
  • Get a approval from another maintainer and I might change my mind
• If we could restrict this to paths below rc.Container.GetActPath() and the workdir, I would pass your change (* I have no rights for getting your change merged).

I'm awaiting the opinion of the other maintainers if they have any, either way you need another review

[ChristopherHX]

Side note bug 2009 in actions/runner also technically allows endless recursion via local composite actions. I assume act has the same bug, however for both local and remote composite actions.

[github-actions]

PR is stale and will be closed in 14 days unless there is new activity

[jenseng]

I’d still like to see this reviewed and merged if possible, I have various actions that use this pattern, and I can’t currently run them under act. I’ll update the PR to resolve the conflicts

[mergify]

@jenseng this pull request has failed checks 🛠

[mergify]

@jenseng this pull request has failed checks 🛠

[jenseng]

🤔 lint failure seems like a bug in golangci/golangci-lint-action when using only-new-issues: true. i'll see if i can work around or get a fix made upstream.

i.e. using golangci-lint@1.53, i get the same failure locally if i do:
golangci-lint run --new-from-patch 0001-Allow-local-actions-outside-the-workspace.patch

or:
golangci-lint run --new-from-rev HEAD

but not if i do:
golangci-lint run pkg/runner

[ChristopherHX]

Yes only me again, one maintainer left (at least made the information available) after my last message here
So we are only cplee (owner, mostly reviewer only, not really active), wolfogre (Gitea Maintainer having their own fork, not really active) and me (have my fork as well).
Getting a review by them is out of my control, or it is unfinished and not visible.

My concern option B

If we could restrict this to paths below rc.Container.GetActPath() and the workdir, I would pass your change (* I have no rights for getting your change merged).

has not been resolved, so not my turn (I can technically elevate my permissings to merge PR's, but I would only do it if act is abandoned by the owner for an extended amount of time, but even then I have another program doing similar things like act as my own little project)

The stale bot seems at this state a bit rough

I did downgrade the go syntax in one of my PR's once due to this linter, but your case is different

Updating the linter using the version field would pull new lint errors of newer stricter rules I don't really like deal with.
I generally have my problems with the golangci linter and coverage requirements

[jsoref]

I'm not a member, but this fixes my test case: https://github.com/check-spelling-sandbox/nektos-act-issue-up-dir, and I see no reason it wouldn't fix my problems with the github/codeql-action repository.

It seems like a reasonable and correct change (to match a behavior upon which GitHub's devs rely).

[github-actions]

PR is stale and will be closed in 14 days unless there is new activity

[jsoref]

Fwiw, I've switched check-spelling to rely on this approach for actions...

If there's a way I can help, I'm somewhat available.

[jenseng]

Sorry, this slipped through the cracks, I'll resolve the conflicts and freshen this up.

@ChristopherHX restricting it to paths below rc.Container.GetActPath() and the workdir would solve my original issue (since I'm referencing other downloaded actions), but it sounds like that wouldn't work for GitHub's own codeql-action (and perhaps not for @jsoref's actions?) since it relies on ./../action (a directory it creates)

What if we restricted to paths below rc.Container.GetActPath() and the workdir's parent directory? That would still constrain it somewhat, while allowing these kinds of use cases to generally work?

[panekj]

imo, we should remain bug-compatible with github if they allow something that shouldn't be done, but library downstreams should be aware of security implications this causes

[ChristopherHX]

if ci passes and panekj approves I let this merge like this is

[jsoref]

Fwiw, my future release which I'm hoping to make shortly will need this.

[jenseng]

@ChristopherHX @panekj let me know if there's anything else here that needs tweaking, thanks!

[ChristopherHX]

No known regression in new action backend (where full test suite is not running here), by cherry picking this for testing only.

This is a perfect example that is not going to work in act even if implemented without aligning the folder layout

        uses: actions/github-script/../../../actions/github-script/v7@v7

If you expand this further, you can get the the working directory, other actions and everywhere else as well.

IMO act is the wrong tool for being 100% bug/feature compatible, this is not documented and can be removed at any point of time from GitHub side via a bug fix.

This "approval" does not allow merging this PR without additional approval from one maintainer / owner

[ChristopherHX]

If everything goes right 🙈 this works in GitHub Actions using actions/runner locally

      - name: Checkout code
        uses: actions/checkout@v4
        if: false

      - name: Create Step Summary
        uses: actions/github-script/../../../actions/checkout/v4@v7

Skipping the checkout action, by just disable running it. Only clone the action and call actions/checkout via actions/github-script.