-
Notifications
You must be signed in to change notification settings - Fork 1
Description
Reading through "GitHub Actions Has a Package Manager, and It Might Be the Worst" by Andrew Nesbitt I stumbled across the following
Undocumented resolution semantics. Every package manager documents how dependency resolution works. npm has a spec. Cargo has a spec. Actions resolution is undocumented. The runner source is public, and the entire “resolution algorithm” is in ActionManager.cs. Here’s a simplified version of what it does:
// Simplified from actions/runner ActionManager.cs async Task PrepareActionsAsync(steps) { // Start fresh every time - no caching DeleteDirectory("_work/_actions"); await PrepareActionsRecursiveAsync(steps, depth: 0); } async Task PrepareActionsRecursiveAsync(actions, depth) { if (depth > 10) throw new Exception("Composite action depth exceeded max depth 10"); foreach (var action in actions) { // Resolution happens on GitHub's server - opaque to us var downloadInfo = await GetDownloadInfoFromGitHub(action.Reference); // Download and extract - no integrity verification var tarball = await Download(downloadInfo.TarballUrl); Extract(tarball, $"_actions/{action.Owner}/{action.Repo}/{downloadInfo.Sha}"); // If composite, recurse into its dependencies var actionYml = Parse($"_actions/{action.Owner}/{action.Repo}/{downloadInfo.Sha}/action.yml"); if (actionYml.Type == "composite") { // These nested actions may use mutable tags - we have no control await PrepareActionsRecursiveAsync(actionYml.Steps, depth + 1); } } }
we should try to make sure the resolution algorithm implemented in ghasum aligns with that of the runner, otherwise we might not able to provide adequate security.
As an example, from the top of my head, we don't consider the max. 10 depth. Now this isn't really a (security) problem for ghasum because users can only end up with redundant checksums in their gha.sum file.