mirror of
https://gitea.com/gitea/runner.git
synced 2026-07-08 08:01:37 +08:00
fix: composite nested uses clone token (#1041)
## Summary A nested `uses:` action inside a **local composite action** fails to clone with a bare `authentication required: Unauthorized` (401) when the instance resolves host-less action references against itself (`DEFAULT_ACTIONS_URL = self`). The job fails at the composite **main→post boundary**, even though the composite's own steps and all post-steps report success. ## Root cause `newCompositeRunContext` nils `Config.Secrets` so composite steps do not see job secrets. But the action-clone path in `prepareActionExecutor` sourced its token via `getGitCloneToken` → `Config.GetToken()` → `Config.Secrets`, which is empty inside a composite RunContext. The nested action is therefore cloned anonymously → 401 against the authenticated instance. Two details explain the exact symptom: - It surfaces at the composite **main→post boundary** because the swallowed nested-step error is re-emitted by `common.JobError` at the end of the composite main pipeline. - It carries **no clone URL** because, on a warm action cache, only `r.Fetch` runs (not `PlainClone`), and the go-git fetch error is returned verbatim. ## Fix Source the clone token from `github.Token` instead of `Config.Secrets`. It is preserved across the composite config copy (`Config.Token` / `PresetGitHubContext`) and is identical to `Config.GetToken()` at the top level, so top-level and `act exec` behaviour is unchanged. The `shouldCloneURLUseToken` host gate is kept so the token is never sent to a foreign host. This also aligns the git-clone path with the ActionCache fetch path, which already uses `github.Token`. Reusable workflows are unaffected — their RunContext keeps `Config.Secrets`. ## Before / after Local composite action with a nested `uses:` (+ post step), followed by a marker step. Same workflow, same runner host — only the runner fix differs. | Job step | Before fix | After fix | | --- | --- | --- | | `Run actions/checkout@v6` | ✅ success | ✅ success | | `Run ./.gitea/actions/probe-composite` (nested `uses:` + post) | ❌ **failure** — bare 401 at the main→post boundary | ✅ success | | `Step after composite` | ⊘ **skipped** | ✅ **ran** | | Job result | ❌ **failed** | ✅ **succeeded** | ### Before — boundary log ```text ::endgroup:: ##[error]authentication required: Unauthorized ← bare 401, no clone target Run Post ./.gitea/actions/probe-composite Success - Post ./.gitea/actions/probe-composite ← post steps run and succeed Success - Post actions/checkout@v6 Job failed ``` The composite's own steps and both post-steps report `Success`; the job fails solely on the bare 401 emitted at the main→post boundary, and the next step is skipped. ### After — boundary log ```text Run ./.gitea/actions/probe-composite ...composite steps... Success - ./.gitea/actions/probe-composite Run Marker after composite PASSED composite boundary — no 401 (runner fix confirmed) Success - Marker after composite Job succeeded ``` ## Reproduction `.gitea/actions/probe-composite/action.yml`: ```yaml name: probe-composite runs: using: composite steps: - uses: actions/setup-node@v6 # nested uses → has a post step with: { node-version: 22 } - run: echo "composite inner step OK" shell: bash ``` `.gitea/workflows/probe.yaml`: ```yaml on: [push] jobs: composite-boundary: runs-on: ubuntu-latest steps: - uses: actions/checkout@v6 - uses: ./.gitea/actions/probe-composite - run: echo "step after composite" # skipped before the fix; runs after ``` ## Verification - **Before:** bare 401 at the boundary, reproduced with a `delay=0` tail — rules out a token-TTL / expiry effect; it is a missing credential. - **After:** the composite step succeeds, the step after the composite runs, the job succeeds, and there is no `authentication required` in the log. - A reusable workflow (`workflow_call`) with the same nested `uses:` + post step never hit the 401, which isolated the bug to the composite main/post path. ## Tests Adds `TestStepActionRemoteCloneTokenSurvivesNilSecrets` (`act/runner`): asserts the clone token is forwarded when the RunContext mirrors a composite (`Secrets == nil`, token via `Config.Token`), and that the host gate still withholds the token for a foreign host. Verified to fail without the fix and pass with it. --------- Reviewed-on: https://gitea.com/gitea/runner/pulls/1041 Reviewed-by: Zettat123 <39446+zettat123@noreply.gitea.com> Co-authored-by: Christian Heim <christian@heimdaheim.de> Co-committed-by: Christian Heim <christian@heimdaheim.de>
This commit is contained in:
@@ -114,9 +114,18 @@ func (sar *stepActionRemote) prepareActionExecutor() common.Executor {
|
|||||||
|
|
||||||
actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), sar.Step.UsesHash())
|
actionDir := fmt.Sprintf("%s/%s", sar.RunContext.ActionCacheDir(), sar.Step.UsesHash())
|
||||||
defaultActionURL := sar.RunContext.Config.DefaultActionURL()
|
defaultActionURL := sar.RunContext.Config.DefaultActionURL()
|
||||||
token := getGitCloneToken(sar.getRunContext().Config, sar.remoteAction.CloneURL(defaultActionURL))
|
// For Gitea
|
||||||
|
// A composite RunContext nils Config.Secrets, so getGitCloneToken would yield an
|
||||||
|
// empty token and clone the action anonymously (401 against the authenticated
|
||||||
|
// instance). github.Token survives the composite config copy and matches the
|
||||||
|
// top-level token; keep the shouldCloneURLUseToken host gate to avoid leaking it.
|
||||||
|
cloneURL := sar.remoteAction.CloneURL(defaultActionURL)
|
||||||
|
token := ""
|
||||||
|
if shouldCloneURLUseToken(sar.RunContext.Config.GitHubInstance, cloneURL) {
|
||||||
|
token = github.Token
|
||||||
|
}
|
||||||
gitClone := stepActionRemoteNewCloneExecutor(git.NewGitCloneExecutorInput{
|
gitClone := stepActionRemoteNewCloneExecutor(git.NewGitCloneExecutorInput{
|
||||||
URL: sar.remoteAction.CloneURL(defaultActionURL),
|
URL: cloneURL,
|
||||||
Ref: sar.remoteAction.Ref,
|
Ref: sar.remoteAction.Ref,
|
||||||
Dir: actionDir,
|
Dir: actionDir,
|
||||||
Token: token,
|
Token: token,
|
||||||
|
|||||||
@@ -838,3 +838,83 @@ func Test_safeFilename(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Regression: a nested action in a composite cloned anonymously (401) because the
|
||||||
|
// composite RunContext nils Config.Secrets. The token must come from github.Token,
|
||||||
|
// which survives the config copy; the host gate must still withhold it cross-host.
|
||||||
|
func TestStepActionRemoteCloneTokenSurvivesNilSecrets(t *testing.T) {
|
||||||
|
const wantToken = "job-token"
|
||||||
|
|
||||||
|
table := []struct {
|
||||||
|
name string
|
||||||
|
gitHubInstance string
|
||||||
|
defaultActionInstance string
|
||||||
|
wantCloneToken string
|
||||||
|
}{
|
||||||
|
{
|
||||||
|
name: "same host forwards token despite nil secrets",
|
||||||
|
gitHubInstance: "gitea.example.com",
|
||||||
|
wantCloneToken: wantToken,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "foreign host is not given the token",
|
||||||
|
gitHubInstance: "gitea.example.com",
|
||||||
|
defaultActionInstance: "github.com",
|
||||||
|
wantCloneToken: "",
|
||||||
|
},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tt := range table {
|
||||||
|
t.Run(tt.name, func(t *testing.T) {
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
var capturedToken string
|
||||||
|
origStepAtionRemoteNewCloneExecutor := stepActionRemoteNewCloneExecutor
|
||||||
|
stepActionRemoteNewCloneExecutor = func(input git.NewGitCloneExecutorInput) common.Executor {
|
||||||
|
capturedToken = input.Token
|
||||||
|
return func(ctx context.Context) error { return nil }
|
||||||
|
}
|
||||||
|
defer (func() {
|
||||||
|
stepActionRemoteNewCloneExecutor = origStepAtionRemoteNewCloneExecutor
|
||||||
|
})()
|
||||||
|
|
||||||
|
sarm := &stepActionRemoteMocks{}
|
||||||
|
sar := &stepActionRemote{
|
||||||
|
Step: &model.Step{Uses: "org/repo@v1"},
|
||||||
|
RunContext: &RunContext{
|
||||||
|
Config: &Config{
|
||||||
|
GitHubInstance: tt.gitHubInstance,
|
||||||
|
DefaultActionInstance: tt.defaultActionInstance,
|
||||||
|
ActionCacheDir: "/tmp/test-cache",
|
||||||
|
// Mirrors the state of a composite RunContext: job secrets are
|
||||||
|
// stripped, but the job token is still reachable via Config.Token.
|
||||||
|
Secrets: nil,
|
||||||
|
Token: wantToken,
|
||||||
|
},
|
||||||
|
Run: &model.Run{
|
||||||
|
JobID: "1",
|
||||||
|
Workflow: &model.Workflow{
|
||||||
|
Jobs: map[string]*model.Job{"1": {}},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
StepResults: map[string]*model.StepResult{},
|
||||||
|
},
|
||||||
|
readAction: sarm.readAction,
|
||||||
|
}
|
||||||
|
sar.RunContext.ExprEval = sar.RunContext.NewExpressionEvaluator(ctx)
|
||||||
|
|
||||||
|
suffixMatcher := func(suffix string) any {
|
||||||
|
return mock.MatchedBy(func(actionDir string) bool {
|
||||||
|
return strings.HasSuffix(actionDir, suffix)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
sarm.On("readAction", sar.Step, suffixMatcher(sar.Step.UsesHash()), "", mock.Anything, mock.Anything).Return(&model.Action{}, nil)
|
||||||
|
|
||||||
|
err := sar.prepareActionExecutor()(ctx)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, tt.wantCloneToken, capturedToken)
|
||||||
|
|
||||||
|
sarm.AssertExpectations(t)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user