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: 878 lines in your changes are missing coverage. Please review.

Comparison is base (4989f44) 61.22% compared to head (3db5ca9) 61.64%.
Report is 277 commits behind head on master.

Files	Patch %	Lines
pkg/artifactcache/handler.go	65.46%	102 Missing and 42 partials ⚠️
pkg/runner/run_context.go	73.37%	75 Missing and 19 partials ⚠️
pkg/runner/expression.go	55.17%	66 Missing and 12 partials ⚠️
pkg/runner/action_cache.go	50.74%	49 Missing and 17 partials ⚠️
pkg/container/docker_network.go	0.00%	56 Missing ⚠️
pkg/container/docker_run.go	1.92%	50 Missing and 1 partial ⚠️
pkg/model/workflow.go	43.37%	40 Missing and 7 partials ⚠️
pkg/common/outbound_ip.go	0.00%	44 Missing ⚠️
pkg/container/host_environment.go	0.00%	43 Missing ⚠️
pkg/artifactcache/storage.go	60.22%	24 Missing and 11 partials ⚠️
... and 25 more

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2108      +/-   ##
==========================================
+ Coverage   61.22%   61.64%   +0.41%     
==========================================
  Files          46       53       +7     
  Lines        7141     8801    +1660     
==========================================
+ Hits         4372     5425    +1053     
- Misses       2462     2947     +485     
- Partials      307      429     +122     

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

[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.