From 4163dd9af9b5ac71ad202af2d6cea04386e9defb Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Wed, 6 May 2026 08:24:31 -0700 Subject: [PATCH] sourcehut: surface log fetch authorization failures Previously the docs only required builds.sr.ht/JOBS:RW, but the log endpoints under /query/log/[/]/log require builds.sr.ht/LOGS:RO. A token with the documented scope would submit jobs and poll status correctly, then 403 on every log fetch. The provider's streamStep suppressed all non-404 errors at debug level and emitted empty steps, so operators following the docs got green builds whose logs were silently blank. Update the docs to require both scopes and explain why. Add an ErrUnauthorized sentinel to the sourcehut client and wrap 401/403 responses from GetTaskLog with it. In the provider, probe the master log up front in Logs() so a systemic auth failure surfaces as a Logs() error (5xx via the HTTP layer) rather than as an empty stream. As defense in depth, streamStep now aborts the stream and logs at error level on ErrUnauthorized instead of swallowing it. --- docs/sourcehut.md | 11 ++++++++--- internal/sourcehut/sourcehut.go | 20 ++++++++++++++++++++ provider_sourcehut.go | 31 +++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 3 deletions(-) diff --git a/docs/sourcehut.md b/docs/sourcehut.md index 75e8d52..21c32e2 100644 --- a/docs/sourcehut.md +++ b/docs/sourcehut.md @@ -13,9 +13,14 @@ each transition. | `TACK_SOURCEHUT_TOKEN` | Personal access token for builds.sr.ht (enables provider) | | `TACK_SOURCEHUT_INSTANCE` | Base URL override (default `https://builds.sr.ht`) | -Generate a token at `https://meta.sr.ht/oauth2/personal-token` with -`builds.sr.ht/JOBS:RW` access, set the `TACK_SOURCEHUT_TOKEN=$token` env var, -then start tack. +Generate a token at `https://meta.sr.ht/oauth2/personal-token` with both +`builds.sr.ht/JOBS:RW` and `builds.sr.ht/LOGS:RO` access, set the +`TACK_SOURCEHUT_TOKEN=$token` env var, then start tack. + +`JOBS:RW` is required to submit jobs and poll their status. `LOGS:RO` +is required because tack fetches per-task logs over an authenticated +HTTP endpoint when serving the appview's log stream — without it the +build will run to completion but every log request will fail. ## Workflow YAML diff --git a/internal/sourcehut/sourcehut.go b/internal/sourcehut/sourcehut.go index 1669817..402aece 100644 --- a/internal/sourcehut/sourcehut.go +++ b/internal/sourcehut/sourcehut.go @@ -27,6 +27,14 @@ const DefaultBaseURL = "https://builds.sr.ht" // for the /logs handler. var ErrNotFound = errors.New("sourcehut: not found") +// ErrUnauthorized is returned by Get* methods when the upstream rejects +// the request with 401/403 — most commonly because the configured +// personal access token is missing the requisite scope (e.g. the log +// endpoints require `builds.sr.ht/LOGS:RO`). It's distinct from +// ErrNotFound so callers can surface a systemic auth misconfiguration +// instead of silently treating it as "no logs yet". +var ErrUnauthorized = errors.New("sourcehut: unauthorized") + // Client is a thin wrapper around net/http carrying API credentials // and the target instance base URL. Safe for concurrent use; the // embedded http.Client is goroutine-safe. @@ -199,6 +207,18 @@ func (c *Client) GetTaskLog(ctx context.Context, jobID int64, taskName string) ( if resp.StatusCode == http.StatusNotFound { return "", ErrNotFound } + if resp.StatusCode == http.StatusUnauthorized || + resp.StatusCode == http.StatusForbidden { + // Surface auth failures distinctly so callers can stop the + // log stream loudly rather than silently emitting empty + // steps for every task. The most common cause is a token + // missing `builds.sr.ht/LOGS:RO`. + raw, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) + return "", fmt.Errorf("get task log: status %d: %s: %w", + resp.StatusCode, strings.TrimSpace(string(raw)), + ErrUnauthorized, + ) + } if resp.StatusCode != http.StatusOK { raw, _ := io.ReadAll(io.LimitReader(resp.Body, 4096)) return "", fmt.Errorf("get task log: status %d: %s", diff --git a/provider_sourcehut.go b/provider_sourcehut.go index e060034..c07b03a 100644 --- a/provider_sourcehut.go +++ b/provider_sourcehut.go @@ -383,6 +383,25 @@ func (p *sourcehutProvider) Logs( return nil, fmt.Errorf("get sourcehut job: %w", err) } + // Probe the master log up front to detect a systemic + // authorization failure (typically a token that lacks + // `builds.sr.ht/LOGS:RO`). Without this check, every per-task + // fetch in the goroutine below would 403 and `streamStep` would + // emit empty steps — leaving the operator with builds whose + // status updates correctly but whose logs are silently blank. + // ErrNotFound here is fine: the runner just hasn't produced any + // setup output yet. + if _, err := client.GetTaskLog(ctx, ref.JobID, ""); err != nil && + !errors.Is(err, sourcehut.ErrNotFound) { + if errors.Is(err, sourcehut.ErrUnauthorized) { + return nil, fmt.Errorf( + "sourcehut log fetch unauthorized; "+ + "token likely missing `builds.sr.ht/LOGS:RO`: %w", err, + ) + } + return nil, fmt.Errorf("probe sourcehut task log: %w", err) + } + out := make(chan LogLine, 32) go func() { defer close(out) @@ -429,6 +448,18 @@ func (p *sourcehutProvider) streamStep( } body, err := client.GetTaskLog(ctx, ref.JobID, logTask) if err != nil && !errors.Is(err, sourcehut.ErrNotFound) { + if errors.Is(err, sourcehut.ErrUnauthorized) { + // Auth errors are systemic — every subsequent task fetch + // will fail the same way. Log loudly and abort the stream + // so the operator notices instead of getting a build with + // every step rendered empty. The HTTP layer can't change + // status mid-stream, but at least the server log will + // point straight at the misconfigured token. + p.log.Error("sourcehut log fetch unauthorized; aborting stream", + "err", err, "job_id", ref.JobID, "task", logTask, + ) + return false + } // Don't fail the whole stream on one task; emit the end frame // and move on so the renderer at least sees what other tasks // produced. ErrNotFound (no log yet) is treated as an empty body. -- 2.51.2