M api/api_test.go => api/api_test.go +3 -9
@@ 49,23 49,17 @@ func router(t *testing.T, w api.Writer, p authn.Principal) http.Handler {
return r
}
-// testResolver is a resolver New requires but Register does not use. One token
-// store with no tokens is enough to construct it.
+// testResolver is a resolver New requires but Register does not use. These
+// tests inject their principal directly, so it needs no agent plane.
func testResolver(t *testing.T) *authn.Resolver {
t.Helper()
- res, err := authn.NewResolver("bigbes", stubTokens{})
+ res, err := authn.NewResolver("bigbes")
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
return res
}
-type stubTokens struct{}
-
-func (stubTokens) LookupAgentToken(context.Context, []byte) (authn.AgentToken, error) {
- return authn.AgentToken{}, authn.ErrUnknownToken
-}
-
func agent() authn.Principal {
return authn.Principal{Kind: authn.KindAgent, Owner: "bigbes", Agent: "claude-code", Session: "s1"}
}
M api/bearer_test.go => api/bearer_test.go +25 -65
@@ 2,8 2,6 @@ package api_test
import (
"context"
- "crypto/sha256"
- "encoding/hex"
"net/http"
"net/http/httptest"
"os"
@@ 45,31 43,9 @@ network-key=tbuG-7Vh44vrDq1L_HKWkHnWrDOtJhEkPKPiauaLeuk=
}
// ---------------------------------------------------------------------------
-// Fixtures: the two credential planes, in front of the real REST write route.
+// Fixtures: the agent credential plane, in front of the real REST write route.
// ---------------------------------------------------------------------------
-// localTokens is spec's own agent_token table, in memory.
-type localTokens struct {
- rows map[string]authn.AgentToken
- calls int
-}
-
-func newLocalTokens() *localTokens { return &localTokens{rows: map[string]authn.AgentToken{}} }
-
-func (s *localTokens) add(secret, name string) {
- sum := sha256.Sum256([]byte(secret))
- s.rows[hex.EncodeToString(sum[:])] = authn.AgentToken{ID: 1, Name: name, Hash: sum[:]}
-}
-
-func (s *localTokens) LookupAgentToken(_ context.Context, hash []byte) (authn.AgentToken, error) {
- s.calls++
- tok, ok := s.rows[hex.EncodeToString(hash)]
- if !ok {
- return authn.AgentToken{}, authn.ErrUnknownToken
- }
- return tok, nil
-}
-
// users resolves the instance owner to a local row.
type users struct{}
@@ 92,12 68,11 @@ func sealToken(grantString string, expires time.Time) string {
func liveToken(grantString string) string { return sealToken(grantString, time.Now().Add(time.Hour)) }
// planeFixture is the REST write plane behind the real resolver middleware,
-// with both credential planes wired: the tokens.sr.ht validator pointed at a
-// fake revocation daemon, and the local agent_token store.
+// with the agent credential plane wired: the tokens.sr.ht validator pointed at
+// a fake revocation daemon.
type planeFixture struct {
handler http.Handler
writer *fakeWriter
- local *localTokens
}
func newPlaneFixture(t *testing.T, daemonStatus int, closeDaemon bool) *planeFixture {
@@ 116,8 91,7 @@ func newPlaneFixture(t *testing.T, daemonStatus int, closeDaemon bool) *planeFix
v, err := bearer.New(bearer.Options{Origin: origin, ClientID: "spec.sr.ht", NodeID: "spec-test"})
require.NoError(t, err)
- local := newLocalTokens()
- resolver, err := authn.NewResolver("bigbes", local, authn.WithInstancePlane(v, users{}))
+ resolver, err := authn.NewResolver("bigbes", authn.WithInstancePlane(v, users{}))
require.NoError(t, err)
w := &fakeWriter{res: service.ProposeResult{
@@ 127,7 101,7 @@ func newPlaneFixture(t *testing.T, daemonStatus int, closeDaemon bool) *planeFix
srv, err := api.New(api.Options{Writer: w, Resolver: resolver})
require.NoError(t, err)
- return &planeFixture{handler: srv.Handler(), writer: w, local: local}
+ return &planeFixture{handler: srv.Handler(), writer: w}
}
func (f *planeFixture) put(token string) *httptest.ResponseRecorder {
@@ 145,22 119,15 @@ func (f *planeFixture) put(token string) *httptest.ResponseRecorder {
// ---------------------------------------------------------------------------
-// The credential every agent on the instance is configured with today still
-// reaches the write plane, with the tokens.sr.ht plane wired in front of it.
-func TestPutWithTheLocalAgentTokenStillWorks(t *testing.T) {
+// The credential every agent on the instance used to be configured with — an
+// opaque secret out of agent_token — reaches nothing any more. 401 at the door,
+// and the write plane never sees the request.
+func TestPutWithTheOldOpaqueAgentTokenIs401(t *testing.T) {
f := newPlaneFixture(t, http.StatusNoContent, false)
- f.local.add("live-token", "laptop")
rec := f.put("live-token")
- require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body)
-
- p := f.writer.got.Principal
- assert.True(t, p.IsAgent())
- assert.Equal(t, authn.PlaneLocal, p.Plane)
- assert.Equal(t, "laptop", p.TokenName)
- assert.Equal(t, "claude-code/spec-writer", p.Agent)
- assert.NoError(t, p.Authorize(authn.ActionPropose),
- "the local plane carries no grants and is refused none")
+ assert.Equal(t, http.StatusUnauthorized, rec.Code)
+ assert.Empty(t, f.writer.got.Writes, "a refused credential must not reach the write plane")
}
// A tokens.sr.ht working token reaches the same route, and arrives carrying the
@@ 177,7 144,6 @@ func TestPutWithAnInstanceToken(t *testing.T) {
assert.Equal(t, "bigbes", p.Owner, "the agent still acts for the instance owner")
assert.Equal(t, 1, p.UserID)
assert.NoError(t, p.Authorize(authn.ActionPropose))
- assert.Zero(t, f.local.calls, "an accepted instance token must not reach the local store")
}
// A narrow token still authenticates here — the resolver runs before the router
@@ 192,38 158,32 @@ func TestPutWithAnInstanceTokenMissingTheProposeGrant(t *testing.T) {
assert.ErrorIs(t, f.writer.got.Principal.Authorize(authn.ActionPropose), authn.ErrMissingGrant)
}
-// A revoked instance token is 401 at the door and never gets a second chance at
-// the old one — the same secret is registered locally, so a fall-through would
-// visibly succeed with a 201.
-func TestPutWithARevokedInstanceTokenIs401AndDoesNotFallThrough(t *testing.T) {
+// A revoked instance token is 401 at the door. There is no second door for it
+// to be re-tried at any more, which is what removing the local plane bought.
+func TestPutWithARevokedInstanceTokenIs401(t *testing.T) {
f := newPlaneFixture(t, http.StatusNotFound, false)
- tok := liveToken("spec:propose id:42")
- f.local.add(tok, "shadow")
- rec := f.put(tok)
+ rec := f.put(liveToken("spec:propose id:42"))
assert.Equal(t, http.StatusUnauthorized, rec.Code)
- assert.Zero(t, f.local.calls, "a revoked instance token must not reach the local store")
assert.Empty(t, f.writer.got.Writes, "and must not reach the write plane")
}
-// An unreachable tokens.sr.ht is 503, never 401 and never a downgrade to the
-// legacy plane: refusing every live instance token because a daemon that is
-// deliberately off the hot path is restarting is the outcome the 503 exists to
-// prevent.
+// An unreachable tokens.sr.ht is 503 and never 401: refusing every live token
+// because a daemon that is deliberately off the hot path is restarting is the
+// outcome the 503 exists to prevent.
func TestPutWithAnUnreachableDaemonIs503(t *testing.T) {
f := newPlaneFixture(t, http.StatusNoContent, true)
- tok := liveToken("spec:propose id:42")
- f.local.add(tok, "shadow")
- rec := f.put(tok)
+ rec := f.put(liveToken("spec:propose id:42"))
assert.Equal(t, http.StatusServiceUnavailable, rec.Code)
- assert.Zero(t, f.local.calls)
assert.Empty(t, f.writer.got.Writes)
}
-// A token from another issuer — a meta.sr.ht PAT — is not this plane's to
-// refuse, so it falls through to the local store, which does not know it.
-func TestPutWithAForeignTokenFallsThroughAndIsRefusedLocally(t *testing.T) {
+// A token from another issuer — a meta.sr.ht PAT — used to fall through to
+// spec's own store, which did not know it. With one plane it is refused where
+// it is presented, and still with a 401: bearer.ErrNotOurs is a permanent
+// refusal, not the 503 an unclassified error would earn.
+func TestPutWithAForeignTokenIs401(t *testing.T) {
f := newPlaneFixture(t, http.StatusNoContent, false)
pat := &auth.BearerToken{
Version: auth.TokenVersion,
@@ 235,5 195,5 @@ func TestPutWithAForeignTokenFallsThroughAndIsRefusedLocally(t *testing.T) {
rec := f.put(pat.Encode())
assert.Equal(t, http.StatusUnauthorized, rec.Code)
- assert.Equal(t, 1, f.local.calls, "the local plane is what refuses a foreign token")
+ assert.Empty(t, f.writer.got.Writes)
}
M authn/authn_test.go => authn/authn_test.go +0 -56
@@ 1,17 1,13 @@
package authn
import (
- "context"
"crypto/rand"
"encoding/base64"
- "encoding/hex"
"encoding/json"
- "fmt"
"net/http"
"net/http/httptest"
"os"
"testing"
- "time"
"github.com/fernet/fernet-go"
"github.com/vaughan0/go-ini"
@@ 120,58 116,6 @@ func testInstance(t *testing.T) Instance {
return inst
}
-// stubStore is an in-memory TokenStore keyed by hex(sha256(token)). It can be
-// told to fail with an arbitrary error to exercise the transient path, and
-// counts calls so tests can assert the cookie path never touches it.
-type stubStore struct {
- rows map[string]AgentToken
- err error
- calls int
-}
-
-func newStubStore() *stubStore { return &stubStore{rows: map[string]AgentToken{}} }
-
-func (s *stubStore) LookupAgentToken(ctx context.Context, hash []byte) (AgentToken, error) {
- s.calls++
- if s.err != nil {
- return AgentToken{}, s.err
- }
- tok, ok := s.rows[hex.EncodeToString(hash)]
- if !ok {
- // The contract db/ must honour: no row means ErrUnknownToken.
- return AgentToken{}, fmt.Errorf("agent_token: %w", ErrUnknownToken)
- }
- return tok, nil
-}
-
-// add stores a live token row for the given secret.
-func (s *stubStore) add(secret, name string) AgentToken {
- hash := HashToken(secret)
- tok := AgentToken{
- ID: int64(len(s.rows) + 1),
- Name: name,
- Hash: hash,
- Created: time.Date(2026, 7, 22, 10, 0, 0, 0, time.UTC),
- }
- s.rows[hex.EncodeToString(hash)] = tok
- return tok
-}
-
-// revoke stores a revoked token row for the given secret.
-func (s *stubStore) revoke(secret, name string) AgentToken {
- tok := s.add(secret, name)
- when := time.Date(2026, 7, 22, 11, 0, 0, 0, time.UTC)
- tok.Revoked = &when
- s.rows[hex.EncodeToString(tok.Hash)] = tok
- return tok
-}
-
-// put stores an arbitrary row under an arbitrary lookup key, for the case where
-// the store returns a row whose hash does not match what was presented.
-func (s *stubStore) put(hash []byte, tok AgentToken) {
- s.rows[hex.EncodeToString(hash)] = tok
-}
-
// request builds a GET / carrying the given cookie value and headers. An empty
// cookie value means no cookie at all.
func request(cookie string, headers map[string]string) *http.Request {
M authn/bearer.go => authn/bearer.go +31 -41
@@ 67,46 67,33 @@ type UserLookup interface {
}
// resolveInstanceToken runs the tokens.sr.ht plane against a presented bearer
-// credential.
+// credential. Its answer is final: there is one agent credential plane, so a
+// refusal here is the service's refusal.
//
-// The middle result says whether the caller should fall back to spec's own
-// agent-token plane. Exactly two refusals fall through, and which two is the
-// only interesting decision in this function:
+// It used to report a third thing — whether the caller should fall back to
+// spec's own agent_token store — and exactly two refusals said yes:
//
-// - bearer.ErrInvalid — the string did not decode as a token this instance
-// sealed. spec's local token is 32 random bytes in base64, which is
-// precisely what that looks like.
-// - bearer.ErrNotOurs — a well-formed token from another issuer (a meta.sr.ht
-// PAT). spec accepts no such credential, but it is not this plane's to
-// refuse, and falling through costs one hash lookup that will miss.
+// - bearer.ErrInvalid, because spec's local token was 32 random bytes in
+// base64, which is precisely what "did not decode as one of ours" looks
+// like;
+// - bearer.ErrNotOurs, because refusing a meta.sr.ht PAT was the local plane's
+// business rather than this one's, and falling through cost one hash lookup
+// that would miss.
//
-// spec's local token carries no prefix to discriminate on — unlike bench's and
-// cover's — so there is no shape test that could route a request to the right
-// plane up front. Trying the instance plane first and falling back on those two
-// sentinels is what replaces it.
+// With that store gone both are plain refusals. The one consequence worth
+// naming is ErrNotOurs: IsAuthFailure now counts it permanent, so a meta PAT
+// presented here earns a 401 rather than the 503 an unclassified error would.
//
-// Every other refusal is terminal and must never reach the old door:
-//
-// - bearer.ErrRevoked — the credential was withdrawn. Letting a revoked
-// instance token be re-tried as a local one would answer "unknown token" for
-// a token an operator deliberately killed, and would mean revocation has a
-// second door to be checked at.
-// - bearer.ErrForbidden — cannot arise from Inspect, which is given no action,
-// but is terminal for the same reason: the credential is good.
-// - bearer.ErrUnavailable — tokens.sr.ht could not be asked. Degrading to the
-// legacy plane when the daemon is unreachable is exactly the silent
-// downgrade the 503 of StatusFor exists to prevent.
+// The rest of the mapping is unchanged and lives in StatusFor: ErrInvalid and
+// ErrRevoked are 401, ErrForbidden and a foreign owner are 403, and
+// ErrUnavailable is 503 — never 401, because "I could not ask tokens.sr.ht" is
+// not "your token is bad".
func (rs *Resolver) resolveInstanceToken(
- ctx context.Context, r *http.Request, presented string,
-) (Principal, bool, error) {
+ ctx context.Context, presented, agent, session string,
+) (Principal, error) {
tok, err := rs.bearer.Inspect(ctx, presented)
- switch {
- case err == nil:
- // fall through
- case errors.Is(err, bearer.ErrInvalid), errors.Is(err, bearer.ErrNotOurs):
- return Anonymous(), true, nil
- default:
- return Anonymous(), false, fmt.Errorf("authn: instance token: %w", err)
+ if err != nil {
+ return Anonymous(), fmt.Errorf("authn: instance token: %w", err)
}
// The token names a meta.sr.ht account, and spec.sr.ht has exactly one that
@@ 122,7 109,7 @@ func (rs *Resolver) resolveInstanceToken(
// doc comment draws between cookies and bearer tokens.
username := strings.TrimPrefix(tok.Username, "~")
if username != rs.owner {
- return Anonymous(), false, fmt.Errorf(
+ return Anonymous(), fmt.Errorf(
"%w: the token belongs to ~%s, and this instance answers only to ~%s",
ErrNotInstanceOwner, username, rs.owner)
}
@@ 135,19 122,19 @@ func (rs *Resolver) resolveInstanceToken(
if err != nil {
// Unclassified, therefore transient, therefore 503: a database that
// cannot answer must never read as a bad credential.
- return Anonymous(), false, fmt.Errorf("authn: resolve instance token owner ~%s: %w", username, err)
+ return Anonymous(), fmt.Errorf("authn: resolve instance token owner ~%s: %w", username, err)
}
return Principal{
Kind: KindAgent,
Owner: rs.owner,
- Agent: strings.TrimSpace(r.Header.Get(HeaderAgent)),
- Session: strings.TrimSpace(r.Header.Get(HeaderAgentSession)),
+ Agent: agent,
+ Session: session,
TokenName: instanceTokenLabel(tok),
Plane: PlaneInstance,
Grants: tok.Grants,
UserID: user.ID,
- }, false, nil
+ }, nil
}
// instanceTokenLabel names the credential in a log line. A registered token has
@@ 176,8 163,11 @@ func instanceTokenLabel(tok *bearer.Token) string {
// - ErrMissingGrant and ErrNotInstanceOwner are 403: the credential verifies
// and the holder is who they say they are, so retrying is pointless and what
// they need is a wider grant, not another login.
-// - Everything permanent about the credential itself — unknown, revoked,
-// malformed, on either plane — is 401.
+// - Everything permanent about the credential itself — malformed, foreign,
+// revoked — is 401.
+// - ErrNoAgentPlane is 503 and not 401. An instance with no [tokens.sr.ht]
+// origin cannot check any credential, and telling the holder of a good token
+// that it is bad would send them to re-provision it.
// - Everything else is transient by definition and answers 503, which is the
// fail-closed direction: a backend outage never reads as a valid credential.
func StatusFor(err error) int {
M authn/bearer_test.go => authn/bearer_test.go +127 -106
@@ 1,7 1,9 @@
package authn
import (
+ "bytes"
"context"
+ "encoding/base64"
"net/http"
"net/http/httptest"
"testing"
@@ 122,11 124,11 @@ type noSuchUserError struct{ username string }
func (e *noSuchUserError) Error() string { return "no such user " + e.username }
-// planeFixture wires a resolver with both planes: the real validator against
-// daemonStatus, and the local agent-token store the old credential lives in.
+// planeFixture wires a resolver with the one agent credential plane: the real
+// ecore validator pointed at a daemon answering daemonStatus, and a stub lookup
+// for the owner a token names.
type planeFixture struct {
rs *Resolver
- store *stubStore
users *stubUsers
daemon *fakeDaemon
}
@@ 134,8 136,8 @@ type planeFixture struct {
func newPlaneFixture(t *testing.T, daemonStatus int) *planeFixture {
t.Helper()
d := newFakeDaemon(t, daemonStatus)
- f := &planeFixture{store: newStubStore(), users: newStubUsers(), daemon: d}
- rs, err := NewResolver("bigbes", f.store, WithInstancePlane(validatorFor(t, d.server.URL), f.users))
+ f := &planeFixture{users: newStubUsers(), daemon: d}
+ rs, err := NewResolver("bigbes", WithInstancePlane(validatorFor(t, d.server.URL), f.users))
require.NoError(t, err)
f.rs = rs
return f
@@ 151,32 153,26 @@ func bearerRequest(token string) *http.Request {
}
// The most important test in this change: the credential every agent on the
-// instance is configured with today still authenticates, still resolves to an
-// agent, and is still authorized to propose — with the instance plane wired in
-// front of it.
-func TestResolve_LocalAgentTokenStillWorksWithTheInstancePlaneWired(t *testing.T) {
+// instance used to be configured with — an opaque 32-byte secret out of
+// agent_token — authenticates nowhere any more. It is not a token this instance
+// sealed, and there is no longer a second store to ask.
+func TestResolve_OldOpaqueAgentTokenIsRefused(t *testing.T) {
f := newPlaneFixture(t, http.StatusNoContent)
- f.store.add("live-token", "laptop")
+ old := base64.RawURLEncoding.EncodeToString(bytes.Repeat([]byte{0x5a}, 32))
- p, err := f.rs.Resolve(context.Background(), bearerRequest("live-token"))
- require.NoError(t, err)
-
- assert.True(t, p.IsAgent(), "the old agent token must still resolve to an agent")
- assert.Equal(t, PlaneLocal, p.Plane)
- assert.Equal(t, "bigbes", p.Owner)
- assert.Equal(t, "laptop", p.TokenName)
- assert.Equal(t, "claude-code/spec-writer", p.Agent)
- assert.Equal(t, "8fb9c9a4-b078-4af1-89eb-d97c522f9921", p.Session)
+ p, err := f.rs.Resolve(context.Background(), bearerRequest(old))
+ require.Error(t, err)
+ assert.ErrorIs(t, err, bearer.ErrInvalid)
+ assert.True(t, p.IsAnonymous(), "a refused credential must yield no authority")
+ assert.Equal(t, http.StatusUnauthorized, StatusFor(err))
- // It carries no grants and is refused nothing: the local plane's boundary is
- // the refs rule, and this change does not move it.
- assert.NoError(t, p.Authorize(ActionPropose))
- assert.NoError(t, p.Authorize(ActionRead))
+ // The push path refuses it for the same reason and through the same call.
+ _, err = f.rs.ResolveAgent(context.Background(), old, "claude-code", "s-1")
+ assert.ErrorIs(t, err, bearer.ErrInvalid)
- // It never became a question for tokens.sr.ht, and never could: the daemon
- // is only asked about a token that decoded as one of its own.
- assert.Zero(t, f.daemon.hits, "the local plane must not talk to tokens.sr.ht")
- assert.Zero(t, f.users.calls, "the local plane has no owner to resolve")
+ _, code, reached := runMiddleware(t, f.rs, bearerRequest(old))
+ assert.False(t, reached)
+ assert.Equal(t, http.StatusUnauthorized, code)
}
func TestResolve_InstanceTokenAccepted(t *testing.T) {
@@ 200,7 196,58 @@ func TestResolve_InstanceTokenAccepted(t *testing.T) {
// A stateless token has no row, so step 4 costs nothing.
assert.Zero(t, f.daemon.hits)
- assert.Zero(t, f.store.calls, "an accepted instance token must not reach the local store")
+}
+
+// ResolveAgent is what the SSH push path calls: the same validator, the same
+// refusals, with the provenance passed as arguments because a hook has no
+// headers to read them from.
+func TestResolveAgent_IsTheSameCheckAsTheHTTPPlane(t *testing.T) {
+ f := newPlaneFixture(t, http.StatusNoContent)
+
+ p, err := f.rs.ResolveAgent(context.Background(), instanceToken("spec:propose"),
+ "claude-code/spec-writer", "s-1")
+ require.NoError(t, err)
+ assert.True(t, p.IsAgent())
+ assert.Equal(t, PlaneInstance, p.Plane)
+ assert.Equal(t, "bigbes", p.Owner)
+ assert.Equal(t, "claude-code/spec-writer", p.Agent)
+ assert.Equal(t, "s-1", p.Session)
+ assert.NoError(t, p.Authorize(ActionPropose))
+ assert.ErrorIs(t, p.Authorize(ActionRead), ErrMissingGrant)
+
+ // No credential at all is ErrNoToken, not an anonymous principal: a caller
+ // that asked to authenticate an agent and passed nothing has a bug.
+ _, err = f.rs.ResolveAgent(context.Background(), "", "claude-code", "s-1")
+ assert.ErrorIs(t, err, ErrNoToken)
+ assert.True(t, IsAuthFailure(err))
+}
+
+// A resolver with no agent plane — an instance whose config.ini has no
+// [tokens.sr.ht] origin — refuses every credential, and does it as a backend
+// failure rather than as a bad token: the holder's credential may be perfect and
+// re-provisioning it would not help.
+func TestResolveAgent_WithoutAPlaneIsAWiringFailure(t *testing.T) {
+ rs, err := NewResolver("bigbes")
+ require.NoError(t, err)
+ assert.False(t, rs.HasInstancePlane())
+
+ tok := instanceToken("spec:propose")
+ p, err := rs.ResolveAgent(context.Background(), tok, "claude-code", "s-1")
+ require.Error(t, err)
+ assert.ErrorIs(t, err, ErrNoAgentPlane)
+ assert.True(t, p.IsAnonymous())
+ assert.False(t, IsAuthFailure(err), "a service that cannot check is not a bad credential")
+ assert.Equal(t, http.StatusServiceUnavailable, StatusFor(err))
+
+ _, code, reached := runMiddleware(t, rs, bearerRequest(tok))
+ assert.False(t, reached)
+ assert.Equal(t, http.StatusServiceUnavailable, code)
+
+ // The cookie plane is unaffected: browsing an instance with no tokens.sr.ht
+ // still works, it just has no agent to serve.
+ owner, err := rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"), nil))
+ require.NoError(t, err)
+ assert.True(t, owner.IsOwner())
}
func TestResolve_InstanceTokenMissingAGrantStillAuthenticates(t *testing.T) {
@@ 216,13 263,12 @@ func TestResolve_InstanceTokenMissingAGrantStillAuthenticates(t *testing.T) {
assert.Equal(t, http.StatusForbidden, StatusFor(p.Authorize(ActionPropose)))
}
-// A revoked instance token must be refused outright and must not get a second
-// chance at the old door. The same string is registered as a local agent token
-// so that a fall-through would visibly succeed.
-func TestResolve_RevokedInstanceTokenDoesNotFallThroughToTheLocalPlane(t *testing.T) {
+// A revoked token is refused outright. There is no second door for it to be
+// re-tried at, which is the property the local plane's removal makes structural
+// rather than merely intended.
+func TestResolve_RevokedInstanceTokenIsRefused(t *testing.T) {
f := newPlaneFixture(t, http.StatusNotFound)
tok := instanceToken("spec:propose id:42")
- f.store.add(tok, "shadow")
p, err := f.rs.Resolve(context.Background(), bearerRequest(tok))
require.Error(t, err)
@@ 231,26 277,22 @@ func TestResolve_RevokedInstanceTokenDoesNotFallThroughToTheLocalPlane(t *testin
assert.Equal(t, http.StatusUnauthorized, StatusFor(err))
assert.True(t, IsAuthFailure(err), "a revoked token is a permanent credential failure")
assert.Equal(t, 1, f.daemon.hits, "a registered token is checked against the daemon")
- assert.Zero(t, f.store.calls, "a revoked instance token must never reach the local store")
_, code, reached := runMiddleware(t, f.rs, bearerRequest(tok))
assert.False(t, reached)
assert.Equal(t, http.StatusUnauthorized, code)
}
-// An unreachable tokens.sr.ht is 503 and never 401, and never a silent
-// downgrade to the legacy plane. Reading "I could not ask" as "revoked" would
-// refuse every live instance token while a daemon that is deliberately off the
-// hot path restarts.
-func TestResolve_UnreachableDaemonIs503AndDoesNotFallThrough(t *testing.T) {
- store := newStubStore()
+// An unreachable tokens.sr.ht is 503 and never 401. Reading "I could not ask"
+// as "revoked" would refuse every live token on the instance while a daemon
+// that is deliberately off the hot path restarts.
+func TestResolve_UnreachableDaemonIs503(t *testing.T) {
users := newStubUsers()
- rs, err := NewResolver("bigbes", store,
+ rs, err := NewResolver("bigbes",
WithInstancePlane(validatorFor(t, unreachableOrigin(t)), users))
require.NoError(t, err)
tok := instanceToken("spec:propose id:42")
- store.add(tok, "shadow")
p, err := rs.Resolve(context.Background(), bearerRequest(tok))
require.Error(t, err)
@@ 258,46 300,41 @@ func TestResolve_UnreachableDaemonIs503AndDoesNotFallThrough(t *testing.T) {
assert.True(t, p.IsAnonymous())
assert.Equal(t, http.StatusServiceUnavailable, StatusFor(err))
assert.False(t, IsAuthFailure(err), "an unreachable daemon is not a bad credential")
- assert.Zero(t, store.calls, "an unanswerable revocation must not fall back to the local store")
_, code, reached := runMiddleware(t, rs, bearerRequest(tok))
assert.False(t, reached)
assert.Equal(t, http.StatusServiceUnavailable, code)
}
-// The two refusals that do fall through. spec's local token has no prefix to
-// discriminate on, so "did not decode as one of ours" is exactly what it looks
-// like — which is why the order is instance-plane-first with a fallback rather
-// than a shape test.
-func TestResolve_ForeignAndUndecodableTokensFallThroughToTheLocalPlane(t *testing.T) {
- metaPAT := seal("bigbes", "meta.sr.ht", "git.sr.ht/OBJECTS:RW", time.Now().Add(time.Hour))
-
- for name, presented := range map[string]string{
- "opaque local secret": "live-token",
- "expired instance token": seal("bigbes", bearer.TokensClientID, "spec:propose",
- time.Now().Add(-time.Hour)),
- "meta.sr.ht PAT": metaPAT,
+// The two refusals that used to fall through to spec's own store. With one
+// plane they are plain 401s — and for the meta PAT that is a change of status
+// as well as of path: ErrNotOurs joined IsAuthFailure when the store it used to
+// be handed to went away, so it answers 401 rather than the 503 an unclassified
+// error would have earned.
+func TestResolve_ForeignAndUndecodableTokensAreRefused(t *testing.T) {
+ for name, c := range map[string]struct {
+ presented string
+ want error
+ }{
+ "opaque secret from the old plane": {"live-token", bearer.ErrInvalid},
+ "expired instance token": {
+ seal("bigbes", bearer.TokensClientID, "spec:propose", time.Now().Add(-time.Hour)),
+ bearer.ErrInvalid,
+ },
+ "meta.sr.ht PAT": {
+ seal("bigbes", "meta.sr.ht", "git.sr.ht/OBJECTS:RW", time.Now().Add(time.Hour)),
+ bearer.ErrNotOurs,
+ },
} {
t.Run(name, func(t *testing.T) {
- t.Run("registered locally", func(t *testing.T) {
- f := newPlaneFixture(t, http.StatusNoContent)
- f.store.add(presented, "laptop")
-
- p, err := f.rs.Resolve(context.Background(), bearerRequest(presented))
- require.NoError(t, err)
- assert.True(t, p.IsAgent())
- assert.Equal(t, PlaneLocal, p.Plane)
- assert.Equal(t, 1, f.store.calls)
- })
-
- t.Run("not registered", func(t *testing.T) {
- f := newPlaneFixture(t, http.StatusNoContent)
-
- _, err := f.rs.Resolve(context.Background(), bearerRequest(presented))
- assert.ErrorIs(t, err, ErrUnknownToken,
- "the local plane must be the one that refuses it")
- assert.Equal(t, http.StatusUnauthorized, StatusFor(err))
- })
+ f := newPlaneFixture(t, http.StatusNoContent)
+
+ p, err := f.rs.Resolve(context.Background(), bearerRequest(c.presented))
+ require.Error(t, err)
+ assert.ErrorIs(t, err, c.want)
+ assert.True(t, p.IsAnonymous())
+ assert.True(t, IsAuthFailure(err), "a credential this service does not take is permanent")
+ assert.Equal(t, http.StatusUnauthorized, StatusFor(err))
})
}
}
@@ 316,7 353,6 @@ func TestResolve_InstanceTokenOfAnotherOwnerIsRefused(t *testing.T) {
assert.True(t, p.IsAnonymous())
assert.Equal(t, http.StatusForbidden, StatusFor(err))
assert.Zero(t, f.users.calls, "a foreign owner is refused before any lookup")
- assert.Zero(t, f.store.calls, "and never falls through to the local plane")
_, code, reached := runMiddleware(t, f.rs, bearerRequest(tok))
assert.False(t, reached)
@@ 334,40 370,23 @@ func TestResolve_UserLookupFailureIs503(t *testing.T) {
assert.Equal(t, http.StatusServiceUnavailable, StatusFor(err))
}
-// An instance with no [tokens.sr.ht] section builds no instance plane, starts,
-// and serves its local agent token exactly as before.
-func TestResolve_WithoutTheInstancePlaneOnlyTheLocalOneExists(t *testing.T) {
- store := newStubStore()
- rs, err := NewResolver("bigbes", store)
- require.NoError(t, err)
- assert.False(t, rs.HasInstancePlane())
-
- store.add("live-token", "laptop")
- p, err := rs.Resolve(context.Background(), bearerRequest("live-token"))
- require.NoError(t, err)
- assert.True(t, p.IsAgent())
- assert.Equal(t, PlaneLocal, p.Plane)
-
- // A perfectly good instance token is just an unknown secret here — there is
- // nothing on this instance that could validate it.
- _, err = rs.Resolve(context.Background(), bearerRequest(instanceToken("spec:propose")))
- assert.ErrorIs(t, err, ErrUnknownToken)
-}
-
func TestWithInstancePlane_RejectsHalfWiring(t *testing.T) {
v := validatorFor(t, "https://tokens.example")
- _, err := NewResolver("bigbes", newStubStore(), WithInstancePlane(nil, newStubUsers()))
+ _, err := NewResolver("bigbes", WithInstancePlane(nil, newStubUsers()))
assert.Error(t, err, "a plane with no validator must be refused")
- _, err = NewResolver("bigbes", newStubStore(), WithInstancePlane(v, nil))
+ _, err = NewResolver("bigbes", WithInstancePlane(v, nil))
assert.Error(t, err, "a plane with no user lookup must be refused")
}
-// Provenance is mandatory on every agent write, on both planes. Grants do not
-// replace it and do not excuse it.
-func TestAgentWriteFor_ProvenanceRequiredOnBothPlanes(t *testing.T) {
+// Provenance is mandatory on every agent write. Grants do not replace it and do
+// not excuse it, and neither does the plane the agent came in on.
+func TestAgentWriteFor_ProvenanceRequired(t *testing.T) {
base := "1f0c1d1a1e2b3c4d5e6f708192a3b4c5d6e7f809"
- for _, plane := range []Plane{PlaneLocal, PlaneInstance} {
- t.Run(string(plane), func(t *testing.T) {
+ for name, plane := range map[string]Plane{
+ "instance token": PlaneInstance,
+ "locally asserted agent": Plane(""),
+ } {
+ t.Run(name, func(t *testing.T) {
complete := Principal{
Kind: KindAgent, Owner: "bigbes", Agent: "a", Session: "s-1",
Plane: plane, Grants: mustGrants(t, "*"),
@@ 392,8 411,9 @@ func TestAuthorize(t *testing.T) {
t.Run("off the instance plane every action passes", func(t *testing.T) {
for _, p := range []Principal{
{Kind: KindOwner, Owner: "bigbes"},
- {Kind: KindAgent, Owner: "bigbes", Plane: PlaneLocal},
- {Kind: KindAgent, Owner: "bigbes"}, // an unset plane is the local one
+ // The CLI's locally asserted agent: no credential, so no grant to
+ // clip. The resolver never produces one of these.
+ {Kind: KindAgent, Owner: "bigbes"},
} {
assert.NoError(t, p.Authorize(ActionPropose))
assert.NoError(t, p.Authorize(ActionRead))
@@ 435,8 455,9 @@ func TestStatusFor(t *testing.T) {
{"bearer forbidden", bearer.ErrForbidden, http.StatusForbidden},
{"revoked instance token", bearer.ErrRevoked, http.StatusUnauthorized},
{"undecodable instance token", bearer.ErrInvalid, http.StatusUnauthorized},
- {"unknown local token", ErrUnknownToken, http.StatusUnauthorized},
- {"revoked local token", ErrRevokedToken, http.StatusUnauthorized},
+ {"token from another issuer", bearer.ErrNotOurs, http.StatusUnauthorized},
+ {"no credential presented", ErrNoToken, http.StatusUnauthorized},
+ {"no agent plane configured", ErrNoAgentPlane, http.StatusServiceUnavailable},
{"store outage", errNoSuchUser("postgres"), http.StatusServiceUnavailable},
}
for _, c := range cases {
M authn/doc.go => authn/doc.go +52 -59
@@ 14,37 14,34 @@
// (agents may only write proposals/*), and that lives in gitx; nothing here
// replaces it.
//
-// # Two agent credential planes
-//
-// An agent is recognised on either of two planes, and Principal.Plane says
-// which:
-//
-// - PlaneLocal — spec's own agent_token row: one instance-wide shared secret,
-// hashed at rest, with no owner, no expiry and no grants. This is v1's
-// credential and it keeps working exactly as it did.
-// - PlaneInstance — a tokens.sr.ht working token: signed by the instance,
-// expiring, owned by a meta.sr.ht account, and carrying a grant set
-// (ActionPropose, ActionRead). It is validated by sr-ht-ecore's bearer
-// package and is present only when the instance config has a
-// [tokens.sr.ht] section; where it does not, the plane is absent and the
-// local one is the only door, which is a supported configuration.
-//
-// The instance plane is tried first and falls back to the local one on exactly
-// two refusals — see Resolver.resolveInstanceToken, where the reasoning lives.
-//
-// The two planes are not interchangeable, and the difference this package has to
-// carry is that only one of them has an owner. The local token is a secret with
-// no user behind it; an instance token names one. Principal.Owner therefore
-// keeps meaning "the human this agent acts for", which on this single-owner
-// instance is always [sr.ht] owner-name — a token belonging to anybody else is
-// refused rather than admitted as a second identity, because every consumer of
-// that field (the provenance committer, the refs rule's principal kind, the
-// coreauth AuthContext) is written for one human.
+// # One agent credential plane
+//
+// An agent is recognised by a tokens.sr.ht working token — PlaneInstance:
+// signed by the instance, expiring, owned by a meta.sr.ht account, and carrying
+// a grant set (ActionPropose, ActionRead). It is validated by sr-ht-ecore's
+// bearer package, and it is the only credential this service authenticates an
+// agent with.
+//
+// spec used to mint its own as well — the agent_token row: one instance-wide
+// shared secret, hashed at rest, with no owner, no expiry and no grants. That
+// plane is gone. Issuance is centralised in tokens.sr.ht, so there is one door
+// and nothing behind it: a credential this plane refuses is refused, rather than
+// being offered to a second store that might say yes. A well-formed token from
+// another issuer (a meta.sr.ht PAT — bearer.ErrNotOurs) used to fall through to
+// that store and now fails at the door, which is the same answer one hash lookup
+// later, said honestly.
+//
+// The instance plane names an owner where the local secret had none.
+// Principal.Owner means "the human this agent acts for", which on this
+// single-owner instance is always [sr.ht] owner-name — a token belonging to
+// anybody else is refused rather than admitted as a second identity, because
+// every consumer of that field (the provenance committer, the refs rule's
+// principal kind, the coreauth AuthContext) is written for one human.
//
// Grants are orthogonal to the refs rule and to provenance, and replace neither.
// A grant says what an instance token was minted for; the refs rule still says
// where an agent may point a ref, and provenance is still mandatory on every
-// agent write, on both planes.
+// agent write.
//
// The cookie and the bearer planes are deliberately asymmetric:
//
@@ 71,13 68,12 @@
// stamped with a guessed session is worse than no commit, because it launders
// unattributable output as attributed.
//
-// This package owns no storage and opens no connections. The agent_token table
-// lives in db/, injected through the TokenStore interface declared here; the
-// "user" row an instance token's owner resolves to is reached through
-// UserLookup, and the tokens.sr.ht validator through BearerValidator. authn
-// never imports db and never calls core-go's auth.LookupUser itself, so the
-// dependency arrow keeps pointing downward and the whole package stays testable
-// with no Postgres and no daemon to talk to.
+// This package owns no storage and opens no connections. The "user" row an
+// instance token's owner resolves to is reached through UserLookup, and the
+// tokens.sr.ht validator through BearerValidator. authn never imports db and
+// never calls core-go's auth.LookupUser itself, so the dependency arrow keeps
+// pointing downward and the whole package stays testable with no Postgres and no
+// daemon to talk to.
package authn
import (
@@ 87,27 83,25 @@ import (
)
// Sentinel errors. Callers compare with errors.Is. The split that matters is
-// permanent (the credential is bad — 401/403) versus transient (the store could
-// not answer — 503); IsAuthFailure draws it.
+// permanent (the credential is bad — 401/403) versus transient (the backend
+// could not answer — 503); IsAuthFailure draws it.
+//
+// Everything the credential itself can be wrong about is now spelled by
+// sr-ht-ecore's bearer package — ErrInvalid, ErrNotOurs, ErrRevoked — because
+// there is one issuer and one validator. The sentinels below are what this
+// service adds on top of that answer.
var (
// ErrNoToken is returned when a bearer credential was expected but the
// request carried no Authorization header, or one in another scheme.
ErrNoToken = errors.New("no agent token presented")
- // ErrUnknownToken is the contract a TokenStore must honour: it is what
- // LookupAgentToken returns (possibly wrapped) when no row matches the
- // presented hash. Any other error is treated as transient, so a Postgres
- // outage reads as "try again", never as "your token is bad".
- ErrUnknownToken = errors.New("unknown agent token")
-
- // ErrInvalidToken marks a presented credential that is malformed, or a
- // stored row whose hash does not actually match what was presented.
- ErrInvalidToken = errors.New("invalid agent token")
-
- // ErrRevokedToken marks a token that resolved to a real row which has been
- // revoked. Distinct from ErrUnknownToken so operators can tell "you are
- // using a token I deliberately killed" from "that token never existed".
- ErrRevokedToken = errors.New("revoked agent token")
+ // ErrNoAgentPlane is returned when a bearer credential is presented to a
+ // resolver that was built without the tokens.sr.ht plane — an instance whose
+ // config.ini has no [tokens.sr.ht] origin. It is a wiring failure and not a
+ // credential failure, so it is deliberately not an IsAuthFailure: telling an
+ // agent its token is bad when the truth is that this service cannot check
+ // any token would send it off to re-provision a perfectly good credential.
+ ErrNoAgentPlane = errors.New("no agent credential plane is configured")
// ErrNotAgent is returned when agent provenance is demanded of a principal
// that is not an agent — the human push path builds no trailers.
@@ 149,19 143,18 @@ var (
// IsAuthFailure reports whether err is a permanent credential failure — the
// caller should answer 401/403 — as opposed to a transient backend failure,
// which should answer 503 and be retried. Everything not in this set is
-// transient by definition, which is the fail-closed direction: a store outage
+// transient by definition, which is the fail-closed direction: a backend outage
// never reads as a valid credential.
//
-// The instance plane's two permanent refusals are in the set for the same
-// reason the local plane's are: a signature that does not verify and a token
-// tokens.sr.ht has withdrawn are both "this credential is bad", whichever door
-// it was presented at. bearer.ErrUnavailable is pointedly absent — see
-// StatusFor, which is what surfaces should map with.
+// bearer.ErrNotOurs joined the set when the local plane left it. A meta.sr.ht
+// PAT used to fall through to spec's own store, where it missed; with one door
+// there is nothing to fall through to, and "that credential was issued by
+// somebody whose tokens this service does not take" is as permanent a refusal as
+// a signature that does not verify. bearer.ErrUnavailable is pointedly absent —
+// see StatusFor, which is what surfaces should map with.
func IsAuthFailure(err error) bool {
return errors.Is(err, ErrNoToken) ||
- errors.Is(err, ErrUnknownToken) ||
- errors.Is(err, ErrInvalidToken) ||
- errors.Is(err, ErrRevokedToken) ||
errors.Is(err, bearer.ErrInvalid) ||
+ errors.Is(err, bearer.ErrNotOurs) ||
errors.Is(err, bearer.ErrRevoked)
}
M authn/principal.go => authn/principal.go +29 -30
@@ 25,27 25,27 @@ const (
// approval, and the only one that may approve a proposal.
KindOwner Kind = "owner"
- // KindAgent is a bot holding an agent bearer token, on either plane. It may
- // propose and it may read; the refs rule in gitx is what stops it touching
- // the approved branch.
+ // KindAgent is a bot holding a tokens.sr.ht working token. It may propose
+ // and it may read; the refs rule in gitx is what stops it touching the
+ // approved branch.
KindAgent Kind = "agent"
)
// Plane names the credential plane an agent authenticated on.
//
-// It exists because the two are not interchangeable and a check that reads
-// Grants has to know whether there were any to read: only the instance plane
-// carries a grant set, and only it names an owner. Empty for every principal
-// that is not an agent.
+// One plane is left, and the field outlives its sibling because the distinction
+// it draws is no longer "which of two stores said yes" but "was there a
+// credential at all". Only a credential carries a grant set, and a check that
+// reads Grants has to know whether there were any to read.
+//
+// Empty for every principal that is not an agent, and for the one agent that is
+// not credential-backed: `specsrht doc propose`, which runs as the operator on
+// the daemon's own host and names an agent for provenance rather than
+// authenticating one. Resolver never produces an agent with an empty plane —
+// every agent it resolves came through the tokens.sr.ht validator.
type Plane string
const (
- // PlaneLocal is spec's own agent_token row: one instance-wide shared secret
- // with no owner, no expiry and no grants. Its whole boundary is the refs
- // rule, which is why v1 shipped it with mandatory provenance instead of
- // scopes.
- PlaneLocal Plane = "local"
-
// PlaneInstance is a tokens.sr.ht working token: signed, expiring, owned by
// a meta.sr.ht account, and carrying the grant set Authorize checks.
PlaneInstance Plane = "instance"
@@ 85,9 85,9 @@ type Principal struct {
Session string
// TokenName names the credential that authenticated this request: the
- // agent_token row's operator-facing label on the local plane, and the
- // tokens.sr.ht row id (or "stateless") on the instance one. KindAgent only,
- // diagnostics only — it grants nothing on either plane.
+ // tokens.sr.ht row id, or "stateless" for a token short enough that the
+ // daemon never wrote it down. KindAgent only, diagnostics only — it grants
+ // nothing.
TokenName string
// CookieUser is whatever username the unified-login cookie carried, even
@@ 100,14 100,13 @@ type Principal struct {
Plane Plane
// Grants is what the instance token this request carried permits, parsed.
- // PlaneInstance only; the zero value on every other plane, which grants
- // nothing and is why Authorize checks Plane before it checks the set.
+ // PlaneInstance only; the zero value everywhere else, which grants nothing
+ // and is why Authorize checks Plane before it checks the set.
Grants grants.Grants
// UserID is the id of the local "user" row the instance token's owner
- // resolved to. PlaneInstance only, and zero on the local plane — which has
- // no owner at all, the asymmetry between the two planes that everything
- // reading this field has to respect.
+ // resolved to. PlaneInstance only, and zero for a principal no credential
+ // backs.
UserID int
}
@@ 146,12 145,12 @@ func (p Principal) CanRead() bool { return p.IsOwner() || p.IsAgent() }
// separate on purpose: the resolver answers identity in middleware, upstream of
// the router, and only the layer that knows the action can ask this one.
//
-// Every plane but PlaneInstance passes, and that is the compatibility contract
-// of this whole change. The local agent token has no grants to check and will
-// not grow any: it is one instance-wide shared secret whose boundary is the refs
-// rule in gitx, and inventing a grant vocabulary for it now would refuse an
-// agent a permission its operator was never asked to give. The owner's cookie
-// passes for the same reason — grants describe machine credentials, not people.
+// A principal off the instance plane passes. That is not a hole left over from
+// the agent_token days: grants describe machine credentials, and the principals
+// with no plane are the owner's cookie — a person, whose authority is their
+// identity — and the CLI's locally asserted agent, which runs as the operator on
+// the daemon's host and presented nothing to have a grant clipped out of. Every
+// agent the resolver produces is on the instance plane and is checked here.
func (p Principal) Authorize(action string) error {
if p.Plane != PlaneInstance {
return nil
@@ 180,9 179,9 @@ func (p Principal) String() string {
session = "(no session)"
}
line := fmt.Sprintf("agent %s session %s for ~%s", agent, session, p.Owner)
- // Only the instance plane is annotated, so the line a local agent logs
- // today reads the same tomorrow — and so that the annotation, when it
- // does appear, means something rather than being noise on every line.
+ // Only a credential-backed agent is annotated: the grant set is what the
+ // annotation says, and an agent a local process asserted has none to
+ // print.
if p.Plane == PlaneInstance {
line += " (tokens.sr.ht: " + p.Grants.String() + ")"
}
M authn/resolver.go => authn/resolver.go +63 -55
@@ 11,28 11,27 @@ import (
)
// Resolver turns a request into a Principal. It holds the instance owner
-// username — the one name a cookie has to match to carry authority — the
-// TokenStore local agent tokens are checked against, and, when the instance is
-// configured for it, the tokens.sr.ht plane.
+// username — the one name a cookie has to match to carry authority — and, when
+// the instance is configured for it, the tokens.sr.ht plane every agent
+// credential is checked against.
type Resolver struct {
owner string
- store TokenStore
- // bearer and users are the instance plane, installed by WithInstancePlane.
- // Both are nil when the instance config has no [tokens.sr.ht] section, which
- // is a supported configuration and not a broken one: the plane is absent and
- // every bearer credential goes straight to the local store, exactly as it
- // did before this plane existed. Resolve tests bearer for presence, and
- // WithInstancePlane is what guarantees the two are wired together or not at
- // all.
+ // bearer and users are the agent plane, installed by WithInstancePlane. Both
+ // are nil when the instance config has no [tokens.sr.ht] section, and a
+ // resolver in that state authenticates no agent at all: since spec stopped
+ // minting its own credential there is nothing else for a bearer token to be
+ // checked against. It is still a legal resolver — the CLI paths that
+ // authenticate nobody build one — but a bearer credential presented to it is
+ // a hard ErrNoAgentPlane, never a shrug.
bearer BearerValidator
users UserLookup
}
// ResolverOption configures a Resolver at construction. Options rather than a
-// second constructor because the instance plane is optional in production and
-// not merely in tests: an instance without tokens.sr.ht must build the same
-// resolver every other caller does.
+// second constructor because the plane is genuinely absent in some processes:
+// `specsrht doc` builds a Service, resolves nobody, and has no use for an HTTP
+// client to tokens.sr.ht.
type ResolverOption func(*Resolver) error
// WithInstancePlane wires the tokens.sr.ht bearer plane in: v validates a
@@ 40,7 39,7 @@ type ResolverOption func(*Resolver) error
//
// Both are required together. A validator with no way to resolve an owner would
// authenticate a token and then have nothing to say about who presented it,
-// which is the one thing the instance plane adds over the local one.
+// which is the whole of what an agent credential is for here.
func WithInstancePlane(v BearerValidator, users UserLookup) ResolverOption {
return func(rs *Resolver) error {
if v == nil {
@@ 58,23 57,16 @@ func WithInstancePlane(v BearerValidator, users UserLookup) ResolverOption {
// NewResolver builds a Resolver for the instance owner named in
// [sr.ht] owner-name.
//
-// A nil store is rejected rather than tolerated: with no store every agent
-// token would resolve as unknown, which looks exactly like a mass revocation
-// and is a miserable thing to debug at 2am. Wire a store or do not build a
-// resolver.
-//
-// With no options the resolver knows only the local agent-token plane — what an
-// instance with no [tokens.sr.ht] section gets, and what every caller got before
-// that plane existed.
-func NewResolver(owner string, store TokenStore, opts ...ResolverOption) (*Resolver, error) {
+// Pass WithInstancePlane to give it an agent plane. Without one it resolves
+// cookies and refuses every bearer credential with ErrNoAgentPlane; the daemon
+// therefore builds one with the plane and fails startup if it cannot, while the
+// CLI paths that authenticate nobody build one without.
+func NewResolver(owner string, opts ...ResolverOption) (*Resolver, error) {
owner = strings.TrimPrefix(owner, "~")
if err := core.ValidateOwner(owner); err != nil {
return nil, fmt.Errorf("authn: instance owner: %w", err)
}
- if store == nil {
- return nil, fmt.Errorf("authn: nil TokenStore")
- }
- rs := &Resolver{owner: owner, store: store}
+ rs := &Resolver{owner: owner}
for _, opt := range opts {
if err := opt(rs); err != nil {
return nil, err
@@ 83,9 75,8 @@ func NewResolver(owner string, store TokenStore, opts ...ResolverOption) (*Resol
return rs, nil
}
-// HasInstancePlane reports whether this resolver tries tokens.sr.ht before the
-// local agent-token store. Startup logging and tests only; never an
-// authorization input.
+// HasInstancePlane reports whether this resolver can authenticate an agent at
+// all. Startup logging and tests only; never an authorization input.
func (rs *Resolver) HasInstancePlane() bool { return rs.bearer != nil }
// Owner returns the instance owner username this resolver recognises.
@@ 99,41 90,27 @@ func (rs *Resolver) Owner() string { return rs.owner }
// it the approved branch. The two credentials are checked in that order and
// never merged.
//
-// A presented bearer token is offered to the tokens.sr.ht plane first, when one
-// is configured, and reaches the local agent-token store only if that plane
-// says the string is not a token of the instance's. Which refusals mean that,
-// and why the order is not the other way round, is in resolveInstanceToken.
+// A presented bearer token goes to the tokens.sr.ht plane and nowhere else.
+// There is no second store behind it since spec stopped minting its own
+// credential, so every refusal that plane returns is final — see
+// resolveInstanceToken.
//
// The error contract is asymmetric on purpose:
//
// - No bearer token: never an error. The cookie decides between KindOwner and
// KindAnonymous, and any cookie problem is anonymity, not failure.
-// - A bearer token that fails: an error. IsAuthFailure separates the 401 case
-// (unknown, revoked, malformed) from the 503 case (store unreachable).
+// - A bearer token that fails: an error. StatusFor separates the 401 case
+// (malformed, foreign, revoked) from the 403 case (a good token this
+// instance has nothing to grant) and the 503 case (tokens.sr.ht
+// unreachable, or no plane wired at all).
//
// The agent identity and session headers are read here but not required: they
// are demanded at the write, by AgentWrite.Validate, which is the only place
// the design requires them and the only place a missing one can do harm.
func (rs *Resolver) Resolve(ctx context.Context, r *http.Request) (Principal, error) {
if presented := BearerFromRequest(r); presented != "" {
- if rs.bearer != nil {
- p, fallBack, err := rs.resolveInstanceToken(ctx, r, presented)
- if !fallBack {
- return p, err
- }
- }
- tok, err := ResolveAgentToken(ctx, rs.store, presented)
- if err != nil {
- return Anonymous(), err
- }
- return Principal{
- Kind: KindAgent,
- Owner: rs.owner,
- Agent: strings.TrimSpace(r.Header.Get(HeaderAgent)),
- Session: strings.TrimSpace(r.Header.Get(HeaderAgentSession)),
- TokenName: tok.Name,
- Plane: PlaneLocal,
- }, nil
+ return rs.ResolveAgent(ctx, presented,
+ r.Header.Get(HeaderAgent), r.Header.Get(HeaderAgentSession))
}
username := UsernameFromRequest(r)
@@ 150,6 127,37 @@ func (rs *Resolver) Resolve(ctx context.Context, r *http.Request) (Principal, er
return Principal{Kind: KindOwner, Owner: username, CookieUser: username}, nil
}
+// ResolveAgent authenticates a presented agent credential, with the provenance
+// the caller collected alongside it, and is what Resolve calls once it has
+// pulled all three out of an HTTP request.
+//
+// It is exported because the push path is not an HTTP request: a `git push`
+// arrives over SSH and the credential reaches the daemon in a hook's
+// environment, not in an Authorization header. That path used to check the
+// agent_token table directly, which is precisely how it ended up unable to
+// accept an instance token while the HTTP surfaces could. One credential plane
+// deserves one implementation of "is this credential good?", so hooks calls this
+// and the two surfaces cannot drift.
+//
+// It never returns an anonymous principal on failure: a presented credential
+// that does not verify is an error, so the caller refuses at the door instead of
+// silently downgrading an agent to a reader.
+func (rs *Resolver) ResolveAgent(ctx context.Context, presented, agent, session string) (Principal, error) {
+ if presented == "" {
+ return Anonymous(), ErrNoToken
+ }
+ if rs.bearer == nil {
+ // Not a bad credential: this process cannot check any credential. 503
+ // via StatusFor, and the operator's clue is in the message rather than
+ // in an agent's incident report about a token that "stopped working".
+ return Anonymous(), fmt.Errorf(
+ "%w: spec.sr.ht authenticates agents through tokens.sr.ht, and this instance's "+
+ "config.ini has no [tokens.sr.ht] origin", ErrNoAgentPlane)
+ }
+ return rs.resolveInstanceToken(ctx, presented,
+ strings.TrimSpace(agent), strings.TrimSpace(session))
+}
+
// Middleware attaches the resolved Principal to the request context, where
// PrincipalFromContext reads it.
//
M authn/resolver_test.go => authn/resolver_test.go +44 -79
@@ 7,29 7,27 @@ import (
"net/http/httptest"
"reflect"
"testing"
+
+ "sourcecraft.dev/bigbes/sr-ht-ecore/bearer"
)
-func newTestResolver(t *testing.T, store TokenStore) *Resolver {
+// newTestResolver builds a resolver whose agent plane is wired to a daemon that
+// answers "live" — the production shape. The cookie tests present no bearer
+// credential and so never reach it.
+func newTestResolver(t *testing.T) *Resolver {
t.Helper()
- rs, err := NewResolver("bigbes", store)
- if err != nil {
- t.Fatalf("NewResolver: %v", err)
- }
- return rs
+ return newPlaneFixture(t, http.StatusNoContent).rs
}
func TestNewResolver_RejectsBadWiring(t *testing.T) {
- if _, err := NewResolver("bigbes", nil); err == nil {
- t.Fatal("nil store must be rejected")
- }
- if _, err := NewResolver("", newStubStore()); err == nil {
+ if _, err := NewResolver(""); err == nil {
t.Fatal("empty owner must be rejected")
}
- if _, err := NewResolver("Not A Name", newStubStore()); err == nil {
+ if _, err := NewResolver("Not A Name"); err == nil {
t.Fatal("unusable owner must be rejected")
}
// The canonical "~user" spelling is accepted and normalised.
- rs, err := NewResolver("~bigbes", newStubStore())
+ rs, err := NewResolver("~bigbes")
if err != nil {
t.Fatalf("NewResolver(~bigbes): %v", err)
}
@@ 39,10 37,9 @@ func TestNewResolver_RejectsBadWiring(t *testing.T) {
}
func TestResolve_OwnerCookie(t *testing.T) {
- store := newStubStore()
- rs := newTestResolver(t, store)
+ f := newPlaneFixture(t, http.StatusNoContent)
- p, err := rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"), nil))
+ p, err := f.rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"), nil))
if err != nil {
t.Fatalf("Resolve: %v", err)
}
@@ 55,13 52,13 @@ func TestResolve_OwnerCookie(t *testing.T) {
if p.Owner != "bigbes" {
t.Fatalf("Owner = %q, want %q", p.Owner, "bigbes")
}
- if store.calls != 0 {
- t.Fatalf("cookie path consulted the token store %d times, want 0", store.calls)
+ if f.users.calls != 0 || f.daemon.hits != 0 {
+ t.Fatal("the cookie path must not touch the agent credential plane")
}
}
func TestResolve_AbsentCookieIsAnonymousNotAnError(t *testing.T) {
- rs := newTestResolver(t, newStubStore())
+ rs := newTestResolver(t)
p, err := rs.Resolve(context.Background(), request("", nil))
if err != nil {
t.Fatalf("an anonymous request must never error: %v", err)
@@ 72,7 69,7 @@ func TestResolve_AbsentCookieIsAnonymousNotAnError(t *testing.T) {
}
func TestResolve_BrokenCookiesAreAnonymousNotErrors(t *testing.T) {
- rs := newTestResolver(t, newStubStore())
+ rs := newTestResolver(t)
for name, value := range map[string]string{
"tampered": tamper(t, sealCookie(t, "bigbes")),
"garbage": "not-a-valid-fernet-token",
@@ 94,7 91,7 @@ func TestResolve_BrokenCookiesAreAnonymousNotErrors(t *testing.T) {
// they read exactly as an anonymous viewer does. The name survives for logs
// only, and must not be mistaken for authority.
func TestResolve_NonOwnerCookieIsAnonymous(t *testing.T) {
- rs := newTestResolver(t, newStubStore())
+ rs := newTestResolver(t)
p, err := rs.Resolve(context.Background(), request(sealCookie(t, "someone"), nil))
if err != nil {
t.Fatalf("Resolve: %v", err)
@@ 114,12 111,10 @@ func TestResolve_NonOwnerCookieIsAnonymous(t *testing.T) {
}
func TestResolve_AgentTokenAccepted(t *testing.T) {
- store := newStubStore()
- store.add("live-token", "laptop")
- rs := newTestResolver(t, store)
+ f := newPlaneFixture(t, http.StatusNoContent)
- p, err := rs.Resolve(context.Background(), request("", map[string]string{
- "Authorization": "Bearer live-token",
+ p, err := f.rs.Resolve(context.Background(), request("", map[string]string{
+ "Authorization": "Bearer " + instanceToken("spec:propose spec:read"),
HeaderAgent: "claude-code/spec-writer",
HeaderAgentSession: "8fb9c9a4-b078-4af1-89eb-d97c522f9921",
}))
@@ 138,47 133,18 @@ func TestResolve_AgentTokenAccepted(t *testing.T) {
if p.Owner != "bigbes" {
t.Fatalf("agent acts for %q, want %q", p.Owner, "bigbes")
}
- if p.TokenName != "laptop" {
- t.Fatalf("TokenName = %q, want %q", p.TokenName, "laptop")
- }
-}
-
-func TestResolve_AgentTokenRevokedAndUnknown(t *testing.T) {
- store := newStubStore()
- store.add("live-token", "laptop")
- store.revoke("dead-token", "cron")
- rs := newTestResolver(t, store)
-
- cases := map[string]struct {
- token string
- want error
- }{
- "revoked": {"dead-token", ErrRevokedToken},
- "unknown": {"never-issued", ErrUnknownToken},
- }
- for name, c := range cases {
- t.Run(name, func(t *testing.T) {
- p, err := rs.Resolve(context.Background(), request("",
- map[string]string{"Authorization": "Bearer " + c.token}))
- if !errors.Is(err, c.want) {
- t.Fatalf("error = %v, want %v", err, c.want)
- }
- if !p.IsAnonymous() {
- t.Fatalf("a refused token must yield no authority: %+v", p)
- }
- })
+ if p.TokenName != "tokens.sr.ht (stateless)" {
+ t.Fatalf("TokenName = %q, want the tokens.sr.ht label", p.TokenName)
}
}
// A bearer token wins over a cookie. Letting a stale browser cookie promote a
// token-bearing request to the owner would hand an agent the approved branch.
func TestResolve_BearerBeatsCookie(t *testing.T) {
- store := newStubStore()
- store.add("live-token", "laptop")
- rs := newTestResolver(t, store)
+ f := newPlaneFixture(t, http.StatusNoContent)
- p, err := rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"),
- map[string]string{"Authorization": "Bearer live-token"}))
+ p, err := f.rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"),
+ map[string]string{"Authorization": "Bearer " + instanceToken("spec:propose")}))
if err != nil {
t.Fatalf("Resolve: %v", err)
}
@@ 194,13 160,12 @@ func TestResolve_BearerBeatsCookie(t *testing.T) {
// agent silently downgraded to a reader fails confusingly at its first write
// instead of clearly at the door.
func TestResolve_BadTokenFailsEvenWithOwnerCookie(t *testing.T) {
- store := newStubStore()
- rs := newTestResolver(t, store)
+ f := newPlaneFixture(t, http.StatusNoContent)
- _, err := rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"),
+ _, err := f.rs.Resolve(context.Background(), request(sealCookie(t, "bigbes"),
map[string]string{"Authorization": "Bearer never-issued"}))
- if !errors.Is(err, ErrUnknownToken) {
- t.Fatalf("error = %v, want ErrUnknownToken", err)
+ if !errors.Is(err, bearer.ErrInvalid) {
+ t.Fatalf("error = %v, want bearer.ErrInvalid", err)
}
}
@@ 218,9 183,7 @@ func runMiddleware(t *testing.T, rs *Resolver, r *http.Request) (Principal, int,
}
func TestMiddleware_AttachesPrincipal(t *testing.T) {
- store := newStubStore()
- store.add("live-token", "laptop")
- rs := newTestResolver(t, store)
+ rs := newTestResolver(t)
t.Run("owner", func(t *testing.T) {
p, code, reached := runMiddleware(t, rs, request(sealCookie(t, "bigbes"), nil))
@@ 244,7 207,7 @@ func TestMiddleware_AttachesPrincipal(t *testing.T) {
t.Run("agent", func(t *testing.T) {
p, code, reached := runMiddleware(t, rs, request("", map[string]string{
- "Authorization": "Bearer live-token",
+ "Authorization": "Bearer " + instanceToken("spec:propose spec:read"),
HeaderAgent: "claude-code/spec-writer",
HeaderAgentSession: "sess-1",
}))
@@ 258,7 221,7 @@ func TestMiddleware_AttachesPrincipal(t *testing.T) {
}
func TestMiddleware_RejectsBadToken(t *testing.T) {
- rs := newTestResolver(t, newStubStore())
+ rs := newTestResolver(t)
_, code, reached := runMiddleware(t, rs, request("",
map[string]string{"Authorization": "Bearer never-issued"}))
if reached {
@@ 269,15 232,16 @@ func TestMiddleware_RejectsBadToken(t *testing.T) {
}
}
-func TestMiddleware_StoreOutageIs503(t *testing.T) {
- store := newStubStore()
- store.err = errors.New("connection refused")
- rs := newTestResolver(t, store)
+// A user lookup that cannot answer is a backend outage, not a bad credential:
+// fail closed with a 503 rather than telling a live agent its token is bad.
+func TestMiddleware_BackendOutageIs503(t *testing.T) {
+ f := newPlaneFixture(t, http.StatusNoContent)
+ f.users.err = errors.New("connection refused")
- _, code, reached := runMiddleware(t, rs, request("",
- map[string]string{"Authorization": "Bearer live-token"}))
+ _, code, reached := runMiddleware(t, f.rs, request("",
+ map[string]string{"Authorization": "Bearer " + instanceToken("spec:read")}))
if reached {
- t.Fatal("a store outage must fail closed, not reach the handler")
+ t.Fatal("a backend outage must fail closed, not reach the handler")
}
if code != http.StatusServiceUnavailable {
t.Fatalf("code = %d, want 503", code)
@@ 336,10 300,11 @@ func TestPrincipal_String(t *testing.T) {
"agent claude-code/spec-writer session s-1 for ~bigbes",
},
{Principal{Kind: KindAgent, Owner: "bigbes"}, "agent (unnamed) session (no session) for ~bigbes"},
- // The local plane renders exactly as it did before the instance plane
- // existed: the annotation appears only where there is something to say.
+ // An agent a local process asserted (the CLI) carries no credential and
+ // so no grant set: the annotation appears only where there is something
+ // to say.
{
- Principal{Kind: KindAgent, Owner: "bigbes", Agent: "a", Session: "s-1", Plane: PlaneLocal},
+ Principal{Kind: KindAgent, Owner: "bigbes", Agent: "a", Session: "s-1"},
"agent a session s-1 for ~bigbes",
},
{
M authn/token.go => authn/token.go +2 -120
@@ 1,15 1,8 @@
package authn
import (
- "context"
- "crypto/sha256"
- "crypto/subtle"
- "database/sql"
- "errors"
- "fmt"
"net/http"
"strings"
- "time"
)
// bearerScheme is the Authorization scheme agents present the token under.
@@ 29,71 22,11 @@ const (
HeaderAgentSession = "X-Agent-Session"
)
-// AgentToken is one row of the agent_token table, in the shape this package
-// needs. It is declared here rather than in db/ so that authn owns its own
-// input contract and the dependency arrow keeps pointing downward.
-type AgentToken struct {
- // ID is the agent_token primary key. Diagnostics and audit only.
- ID int64
-
- // Name is the operator-facing label of the token ("laptop", "cron"). With
- // one token and no scopes it grants nothing; it exists so a revocation can
- // be aimed at something a human recognises.
- Name string
-
- // Hash is the stored sha256 of the token as issued. ResolveAgentToken
- // re-checks it against the presented token in constant time rather than
- // trusting that the store's lookup was an exact match.
- Hash []byte
-
- // Created is when the token was issued.
- Created time.Time
-
- // Revoked is when the token was revoked, or nil while it is live. A
- // revoked row still resolves from the store — it is this package that
- // turns it into a refusal, so the refusal can say "revoked" rather than
- // "unknown".
- Revoked *time.Time
-}
-
-// IsRevoked reports whether the token has been revoked.
-func (t AgentToken) IsRevoked() bool { return t.Revoked != nil }
-
-// TokenStore is the sliver of db/ that authn needs: look up an agent token row
-// by the hash of the presented secret. service/ wires the real Postgres
-// implementation in; tests wire a map.
-//
-// Contract:
-//
-// - hash is the value HashToken returned; the implementation must match it
-// against agent_token.token_hash exactly, never by prefix.
-// - When no row matches, return an error satisfying
-// errors.Is(err, ErrUnknownToken). sql.ErrNoRows is accepted as an
-// equivalent spelling, since that is what a bare QueryRow().Scan() yields.
-// - Any other error is taken to be transient (Postgres down, context
-// cancelled) and is surfaced as such, never as a bad credential.
-//
-// The interface takes no scope or space argument on purpose: v1 has one agent
-// token and no per-space scoping, and the boundary that actually bounds damage
-// is the refs rule in gitx. Adding scopes later is a column here and a filter
-// clause there, not a reshaping of this interface.
-type TokenStore interface {
- LookupAgentToken(ctx context.Context, hash []byte) (AgentToken, error)
-}
-
-// HashToken returns the sha256 of a presented agent token — the value stored in
-// agent_token.token_hash and the only form of the secret this service keeps.
-// The token itself is opaque and high-entropy, so a plain hash is sufficient:
-// there is no low-entropy password here for a KDF to slow down guessing of.
-func HashToken(token string) []byte {
- sum := sha256.Sum256([]byte(token))
- return sum[:]
-}
-
// BearerFromRequest returns the token from an "Authorization: Bearer <token>"
// header, or "" when the header is absent or uses another scheme. Anything
// after the scheme is returned verbatim apart from surrounding whitespace: the
-// token is opaque and this package is not the place to guess at its grammar.
+// token is opaque to this function, and validating its grammar is
+// BearerValidator's job and not the header parser's.
func BearerFromRequest(r *http.Request) string {
h := r.Header.Get("Authorization")
if h == "" {
@@ 105,54 38,3 @@ func BearerFromRequest(r *http.Request) string {
}
return strings.TrimSpace(rest)
}
-
-// ResolveAgentToken validates a presented agent token against the store.
-//
-// Unlike the cookie path this fails loudly. An agent that presented a
-// credential and got silently downgraded to an anonymous reader would sail
-// through its reads and then fail incomprehensibly at its first propose; a 401
-// at the door is the only useful answer.
-//
-// Permanent refusals (ErrNoToken, ErrUnknownToken, ErrRevokedToken,
-// ErrInvalidToken) satisfy IsAuthFailure. A store failure is returned wrapped
-// and does not, so callers answer 503 rather than 401 and agents retry rather
-// than re-provision.
-func ResolveAgentToken(ctx context.Context, store TokenStore, presented string) (AgentToken, error) {
- if store == nil {
- // A nil store is a wiring bug, not a credential problem. Refusing
- // loudly beats resolving every token as unknown, which would look like
- // a revocation storm.
- return AgentToken{}, errors.New("authn: nil TokenStore")
- }
- if presented == "" {
- return AgentToken{}, ErrNoToken
- }
-
- hash := HashToken(presented)
- tok, err := store.LookupAgentToken(ctx, hash)
- switch {
- case err == nil:
- // fall through
- case errors.Is(err, ErrUnknownToken), errors.Is(err, sql.ErrNoRows):
- // Two spellings of "no such row"; ErrUnknownToken is the contract,
- // sql.ErrNoRows is what an unwrapped Scan leaks.
- return AgentToken{}, fmt.Errorf("%w: no agent token matches the presented secret", ErrUnknownToken)
- default:
- return AgentToken{}, fmt.Errorf("looking up agent token: %w", err)
- }
-
- // Re-check the hash ourselves, in constant time. The store's WHERE clause
- // already did an equality match, but this is the one comparison that
- // decides authentication and it costs a memcmp to not depend on somebody
- // else getting it right.
- if subtle.ConstantTimeCompare(tok.Hash, hash) != 1 {
- return AgentToken{}, fmt.Errorf("%w: store returned a row whose hash does not match the presented token", ErrInvalidToken)
- }
-
- if tok.IsRevoked() {
- return AgentToken{}, fmt.Errorf("%w: token %q was revoked at %s",
- ErrRevokedToken, tok.Name, tok.Revoked.UTC().Format(time.RFC3339))
- }
-
- return tok, nil
-}
M authn/token_test.go => authn/token_test.go +0 -131
@@ 1,29 1,11 @@
package authn
import (
- "bytes"
- "context"
- "database/sql"
- "errors"
"net/http"
"net/http/httptest"
"testing"
)
-func TestHashToken_IsSHA256AndStable(t *testing.T) {
- a := HashToken("s3cret")
- b := HashToken("s3cret")
- if len(a) != 32 {
- t.Fatalf("hash length = %d, want 32", len(a))
- }
- if !bytes.Equal(a, b) {
- t.Fatal("hashing the same token twice produced different values")
- }
- if bytes.Equal(a, HashToken("s3cres")) {
- t.Fatal("distinct tokens hashed to the same value")
- }
-}
-
func TestBearerFromRequest(t *testing.T) {
cases := map[string]struct{ header, want string }{
"bearer": {"Bearer abc123", "abc123"},
@@ 46,116 28,3 @@ func TestBearerFromRequest(t *testing.T) {
})
}
}
-
-func TestResolveAgentToken_Accepted(t *testing.T) {
- store := newStubStore()
- want := store.add("live-token", "laptop")
-
- got, err := ResolveAgentToken(context.Background(), store, "live-token")
- if err != nil {
- t.Fatalf("ResolveAgentToken: %v", err)
- }
- if got.ID != want.ID || got.Name != "laptop" {
- t.Fatalf("resolved %+v, want id %d name %q", got, want.ID, "laptop")
- }
- if got.IsRevoked() {
- t.Fatal("live token reported as revoked")
- }
-}
-
-func TestResolveAgentToken_Revoked(t *testing.T) {
- store := newStubStore()
- store.revoke("dead-token", "cron")
-
- _, err := ResolveAgentToken(context.Background(), store, "dead-token")
- if !errors.Is(err, ErrRevokedToken) {
- t.Fatalf("error = %v, want ErrRevokedToken", err)
- }
- // A revoked token must not be reported as unknown: the operator needs to
- // tell "I killed this" from "this never existed".
- if errors.Is(err, ErrUnknownToken) {
- t.Fatalf("revoked token also reported as unknown: %v", err)
- }
- if !IsAuthFailure(err) {
- t.Fatalf("revocation must be a permanent auth failure: %v", err)
- }
-}
-
-func TestResolveAgentToken_Unknown(t *testing.T) {
- store := newStubStore()
- store.add("live-token", "laptop")
-
- _, err := ResolveAgentToken(context.Background(), store, "never-issued")
- if !errors.Is(err, ErrUnknownToken) {
- t.Fatalf("error = %v, want ErrUnknownToken", err)
- }
- if !IsAuthFailure(err) {
- t.Fatalf("unknown token must be a permanent auth failure: %v", err)
- }
-}
-
-// db/ may hand back a bare sql.ErrNoRows from QueryRow().Scan(); it means the
-// same thing as ErrUnknownToken and must not be mistaken for a store outage.
-func TestResolveAgentToken_SQLNoRowsIsUnknown(t *testing.T) {
- store := newStubStore()
- store.err = sql.ErrNoRows
-
- _, err := ResolveAgentToken(context.Background(), store, "whatever")
- if !errors.Is(err, ErrUnknownToken) {
- t.Fatalf("error = %v, want ErrUnknownToken", err)
- }
-}
-
-func TestResolveAgentToken_EmptyIsNoToken(t *testing.T) {
- store := newStubStore()
- _, err := ResolveAgentToken(context.Background(), store, "")
- if !errors.Is(err, ErrNoToken) {
- t.Fatalf("error = %v, want ErrNoToken", err)
- }
- if store.calls != 0 {
- t.Fatalf("store consulted %d times for an absent token, want 0", store.calls)
- }
-}
-
-// A store outage must never read as a bad credential: fail closed, but tell the
-// caller it is transient so it answers 503 and the agent retries.
-func TestResolveAgentToken_StoreFailureIsTransient(t *testing.T) {
- boom := errors.New("connection refused")
- store := newStubStore()
- store.add("live-token", "laptop")
- store.err = boom
-
- _, err := ResolveAgentToken(context.Background(), store, "live-token")
- if !errors.Is(err, boom) {
- t.Fatalf("error = %v, want it to wrap the store error", err)
- }
- if IsAuthFailure(err) {
- t.Fatalf("store failure must not be a permanent auth failure: %v", err)
- }
-}
-
-// The constant-time re-check exists so that a store which matched loosely — by
-// prefix, or on the wrong column — cannot authenticate anybody.
-func TestResolveAgentToken_HashMismatchRejected(t *testing.T) {
- store := newStubStore()
- store.put(HashToken("presented"), AgentToken{
- ID: 7,
- Name: "sloppy-store",
- Hash: HashToken("something-else"),
- })
-
- _, err := ResolveAgentToken(context.Background(), store, "presented")
- if !errors.Is(err, ErrInvalidToken) {
- t.Fatalf("error = %v, want ErrInvalidToken", err)
- }
-}
-
-func TestResolveAgentToken_NilStoreIsNotAnAuthFailure(t *testing.T) {
- _, err := ResolveAgentToken(context.Background(), nil, "live-token")
- if err == nil {
- t.Fatal("nil store must be an error")
- }
- if IsAuthFailure(err) {
- t.Fatalf("a wiring bug must not read as a bad credential: %v", err)
- }
-}
M cmd/specsrht-migrate/main_test.go => cmd/specsrht-migrate/main_test.go +7 -1
@@ 428,10 428,16 @@ func TestUpFromEmpty(t *testing.T) {
if err != nil {
t.Fatalf("db: %v", err)
}
- for _, table := range []string{"space", "document_id", "proposal", "agent_token", "index_stamp", "digest_mark"} {
+ for _, table := range []string{"space", "document_id", "proposal", "index_stamp", "digest_mark"} {
var n int
if err := db.QueryRow(`SELECT count(*) FROM ` + table).Scan(&n); err != nil {
t.Errorf("table %s missing after up: %v", table, err)
}
}
+
+ // And agent_token must be gone: 0005 drops it, and a database that still had
+ // it would still have a second door into the write plane.
+ if err := db.QueryRow(`SELECT count(*) FROM agent_token`).Scan(new(int)); err == nil {
+ t.Error("agent_token survived the migrations; agent credentials come from tokens.sr.ht")
+ }
}
M cmd/specsrht/doc.go => cmd/specsrht/doc.go +5 -3
@@ 36,9 36,11 @@ const docUsage = "usage: specsrht doc propose ~owner/space <file>... [flags]"
// The two agent surfaces are remote and therefore need a bearer token; this one
// is not. It runs on the host, with the repositories and Postgres already in
// hand, and constructs the agent principal directly rather than resolving one
-// from an agent_token row. That is not a hole: a process that can already open
-// the database and the bare repositories can do anything the token would let it
-// do, and demanding a credential from it would only be ceremony. Provenance is
+// from a presented credential. That is not a hole: a process that can already
+// open the database and the bare repositories can do anything a token would let
+// it do, and demanding a credential from it would only be ceremony — which is
+// also why the principal it builds carries no credential plane, and so no grant
+// for authn.Principal.Authorize to check. Provenance is
// *not* waived, though — --agent and --session are recorded exactly as a remote
// agent's are, so a `git log` cannot tell a proposal opened here from one opened
// over HTTP, and neither can a reviewer.
M cmd/specsrht/main.go => cmd/specsrht/main.go +16 -14
@@ 24,18 24,21 @@
//
// # Admin commands
//
-// Three subcommands run and exit without binding anything, so they are safe to
+// Two subcommands run and exit without binding anything, so they are safe to
// invoke while the daemon is up:
//
// specsrht space create ~owner/name | specsrht space list
-// specsrht token create <name> | specsrht token list | specsrht token revoke <id>
// specsrht doc propose ~owner/space <file>... [--as path] [--title t]
//
-// The first two are the only entry points spaces and agent tokens have. A
-// deployment without both is inert: nothing to hold documents, and no credential
-// for the remote agent write plane, which refuses an anonymous caller by design.
-// The third opens a proposal from files on this host, for when the documents and
-// the operator are already here and a bearer token would be ceremony.
+// The first is the only entry point a space has, and a deployment without one
+// is inert: there is nothing to hold documents. The second opens a proposal
+// from files on this host, for when the documents and the operator are already
+// here and a bearer token would be ceremony.
+//
+// There is no `specsrht token`. It minted spec's own agent credential; agent
+// credentials are tokens.sr.ht working tokens now, so they are minted there —
+// its /tokens page, or POST /exchange with a parent token — and carry the
+// spec:propose grant to write and spec:read to read.
//
// # Flags
//
@@ 132,7 135,6 @@ func main() {
if len(os.Args) > 1 {
admin := map[string]func([]string) error{
"space": runSpace,
- "token": runToken,
"doc": runDoc,
}
if cmd := os.Args[1]; admin[cmd] != nil {
@@ 269,16 271,16 @@ func run(log *slog.Logger) error {
}
defer pool.Close()
- // WithInstanceTokens offers the daemon's resolver the tokens.sr.ht bearer
- // plane. It is offered, not required: an instance whose config has no
- // [tokens.sr.ht] section gets no such plane and keeps accepting its own
- // agent token, which is a supported configuration and not a degraded one.
+ // WithInstanceTokens builds the daemon's one agent credential plane. It is
+ // required, not offered: spec.sr.ht mints no credential of its own any more,
+ // so an instance with no [tokens.sr.ht] section could authenticate no agent
+ // at all, over HTTP or over `git push`. service.New fails here rather than
+ // letting the daemon come up and refuse every agent one request at a time.
svc, err := service.New(cfg, pool, service.WithInstanceTokens(conf))
if err != nil {
return err
}
- log.Info("agent credential planes",
- "local", true, "tokens.sr.ht", svc.Resolver().HasInstancePlane())
+ log.Info("agent credential plane", "tokens.sr.ht", svc.Resolver().HasInstancePlane())
// Seed the owner's user row before serving. core-go's auth.Middleware looks
// a request's username up in the "user" table and, on a miss, calls out to
D cmd/specsrht/token.go => cmd/specsrht/token.go +0 -136
@@ 1,136 0,0 @@
-package main
-
-import (
- "context"
- "errors"
- "fmt"
- "os"
- "strconv"
- "text/tabwriter"
- "time"
-
- "sourcecraft.dev/bigbes/sr-ht-core/config"
-
- "sourcecraft.dev/bigbes/sr-ht-spec/authn"
- "sourcecraft.dev/bigbes/sr-ht-spec/service"
-)
-
-const tokenUsage = "usage: specsrht token create <name> | specsrht token list | specsrht token revoke <id>"
-
-// runToken is the agent-token administration command:
-//
-// specsrht token create <name>
-// specsrht token list
-// specsrht token revoke <id>
-//
-// Agent tokens have no other entry point, and without one a freshly deployed
-// instance cannot be written to at all: both agent write surfaces — the REST
-// PUT and mcpsrv's spec_propose — refuse an anonymous caller, and the human
-// path (native receive-pack) is approval rather than proposal. The alternative
-// to this command is an operator hand-writing an INSERT with a sha256 hash,
-// which is exactly the shape of mistake that ends with an unusable credential
-// and no way to tell why.
-//
-// It goes through service/ rather than db/ directly, and constructs the owner
-// principal to do so. The owner-only rule on minting is the one that makes
-// revocation mean anything — an agent that could mint would survive having its
-// credential revoked — so it is spelled once, in service/, and this command is
-// held to it exactly as the /tokens page is. A process on this host could of
-// course write the row itself; the point is not to confine it but to keep one
-// implementation of what issuing a token *is*.
-func runToken(args []string) error {
- if len(args) == 0 {
- return errors.New(tokenUsage)
- }
-
- conf := config.LoadConfig()
- cfg, err := validateConfig(conf)
- if err != nil {
- return err
- }
- pool, err := openDatabase(cfg.ConnectionString)
- if err != nil {
- return err
- }
- defer pool.Close()
-
- svc, err := service.New(cfg, pool)
- if err != nil {
- return err
- }
- owner := authn.Principal{Kind: authn.KindOwner, Owner: cfg.Instance.OwnerName}
- ctx := context.Background()
-
- switch args[0] {
- case "create":
- if len(args) != 2 {
- return errors.New("usage: specsrht token create <name>")
- }
- name := args[1]
-
- // The plaintext is returned by the mint and printed once. Nothing writes
- // it to the log, because a token in a log file is a token in a backup.
- token, row, err := svc.IssueAgentToken(ctx, owner, name)
- if err != nil {
- return err
- }
- fmt.Printf("created agent token %q (id %d)\n\n %s\n\n"+
- "This is the only time the token is shown — only its hash is stored.\n"+
- "Agents present it as: Authorization: Bearer <token>\n",
- row.Name, row.ID, token)
- return nil
-
- case "list":
- tokens, err := svc.ListAgentTokens(ctx, owner)
- if err != nil {
- return err
- }
- w := tabwriter.NewWriter(os.Stdout, 0, 0, 2, ' ', 0)
- fmt.Fprintln(w, "ID\tNAME\tCREATED\tSTATE")
- for _, t := range tokens {
- fmt.Fprintln(w, formatTokenRow(t))
- }
- return w.Flush()
-
- case "revoke":
- if len(args) != 2 {
- return errors.New("usage: specsrht token revoke <id>")
- }
- id, err := parseTokenID(args[1])
- if err != nil {
- return err
- }
- if err := svc.RevokeAgentToken(ctx, owner, id); err != nil {
- return err
- }
- fmt.Printf("revoked agent token %d\n", id)
- return nil
-
- default:
- return fmt.Errorf("unknown subcommand %q: want create, list or revoke", args[0])
- }
-}
-
-// formatTokenRow renders one token as a tab-separated line for `token list`.
-// The hash is deliberately not shown: it identifies nothing an operator acts
-// on, and printing a column of it would only invite treating it as the
-// credential.
-func formatTokenRow(t service.AgentToken) string {
- state := "active"
- if !t.Active() {
- state = "revoked " + t.Revoked.Format(time.RFC3339)
- }
- return fmt.Sprintf("%d\t%s\t%s\t%s", t.ID, t.Name, t.Created.Format(time.RFC3339), state)
-}
-
-// parseTokenID reads the id argument of `token revoke`. It rejects anything
-// that is not a positive integer here rather than letting a typo become an
-// UPDATE that matches no row and reports "not found", which reads like the
-// token is already gone.
-func parseTokenID(s string) (int, error) {
- id, err := strconv.Atoi(s)
- if err != nil || id <= 0 {
- return 0, fmt.Errorf("token id %q is not a positive integer; `specsrht token list` shows the ids", s)
- }
- return id, nil
-}
D cmd/specsrht/token_test.go => cmd/specsrht/token_test.go +0 -70
@@ 1,70 0,0 @@
-package main
-
-import (
- "strings"
- "testing"
- "time"
-
- "sourcecraft.dev/bigbes/sr-ht-spec/service"
-)
-
-func TestParseTokenIDRejectsWhatIsNotAnID(t *testing.T) {
- for _, in := range []string{"", "0", "-1", "3.0", "abc", " 3", "3 "} {
- if _, err := parseTokenID(in); err == nil {
- t.Errorf("parseTokenID(%q) was accepted", in)
- }
- }
- id, err := parseTokenID("42")
- if err != nil {
- t.Fatalf("parseTokenID(\"42\"): %v", err)
- }
- if id != 42 {
- t.Errorf("parseTokenID(\"42\") = %d want 42", id)
- }
-}
-
-// TestParseTokenIDPointsAtTheListing keeps the failure actionable: an operator
-// who mistyped an id needs to be told where the ids come from, not just that
-// this one was wrong.
-func TestParseTokenIDPointsAtTheListing(t *testing.T) {
- _, err := parseTokenID("nope")
- if err == nil {
- t.Fatal("parseTokenID accepted a non-numeric id")
- }
- if !strings.Contains(err.Error(), "specsrht token list") {
- t.Errorf("the error does not say how to find the ids:\n%v", err)
- }
-}
-
-func TestFormatTokenRow(t *testing.T) {
- created := time.Date(2026, 8, 5, 9, 30, 0, 0, time.UTC)
- revoked := created.Add(24 * time.Hour)
-
- active := formatTokenRow(service.AgentToken{ID: 1, Name: "claude", Created: created})
- if want := "1\tclaude\t2026-08-05T09:30:00Z\tactive"; active != want {
- t.Errorf("active row = %q want %q", active, want)
- }
-
- dead := formatTokenRow(service.AgentToken{ID: 2, Name: "old", Created: created, Revoked: &revoked})
- if want := "2\told\t2026-08-05T09:30:00Z\trevoked 2026-08-06T09:30:00Z"; dead != want {
- t.Errorf("revoked row = %q want %q", dead, want)
- }
-}
-
-// There is no test that the listing keeps the stored hash out: service.AgentToken
-// has no hash field, so the type is the guarantee and a test would only assert
-// that Go's struct literals work.
-
-// TestRunTokenRejectsBadInvocationsBeforeTheDatabase guards the ordering that
-// makes this command usable on a workstation: a usage error must be reported
-// without a config file or a Postgres connection, which is what an operator
-// typing it blind has.
-func TestRunTokenRejectsBadInvocationsBeforeTheDatabase(t *testing.T) {
- err := runToken(nil)
- if err == nil {
- t.Fatal("runToken accepted an empty argument list")
- }
- if err.Error() != tokenUsage {
- t.Errorf("empty invocation did not print the usage line:\n%v", err)
- }
-}
M db/store.go => db/store.go +8 -12
@@ 1,7 1,12 @@
// Package db is the PostgreSQL persistence layer for spec.sr.ht. It maps the
-// eight tables of schema.sql — space, document_id, proposal, agent_token,
-// index_stamp, digest_mark, project and project_space — to core value types
-// with plain database/sql and $n placeholders (no ORM).
+// tables of schema.sql — space, document_id, proposal, index_stamp,
+// digest_mark, project, project_space and comment — to core value types with
+// plain database/sql and $n placeholders (no ORM).
+//
+// It holds no credential of any kind. agent_token lived here until agent
+// issuance moved to tokens.sr.ht; a working token is signed rather than stored,
+// so authenticating one is a signature check in authn/ and this package has
+// nothing to look up.
//
// The layering rule from the design is what shapes this package: **git refs are
// the source of truth for whether a proposal exists and whether it merged;
@@ 139,15 144,6 @@ var (
// push itself, not against the registry.
ErrDocIDDuplicate = errors.New("db: document id appears twice in one batch")
- // ErrTokenExists is returned by CreateAgentToken when that exact token is
- // already registered (agent_token.token_hash UNIQUE).
- ErrTokenExists = errors.New("db: agent token already registered")
-
- // ErrTokenRevoked is returned by AuthenticateAgentToken for a token that
- // exists but has been revoked. Kept distinct from ErrNotFound so the audit
- // log can say which happened; both map to 401 at the API boundary.
- ErrTokenRevoked = errors.New("db: agent token revoked")
-
// ErrProposalNotOpen is returned by DeleteOpenProposal for a proposal that
// exists but has already been resolved. Kept distinct from ErrNotFound so
// the reconciler can tell "the row is gone", which is the state it wanted,
D db/token.go => db/token.go +0 -201
@@ 1,201 0,0 @@
-package db
-
-import (
- "context"
- "crypto/rand"
- "crypto/sha256"
- "crypto/subtle"
- "database/sql"
- "encoding/base64"
- "errors"
- "fmt"
- "time"
-
- "github.com/lib/pq"
-)
-
-// TokenBytes is the entropy of a minted agent token before encoding. 32 bytes
-// is well past any brute-force concern and keeps the encoded form short enough
-// to paste into an agent's environment.
-const TokenBytes = 32
-
-// AgentToken is a credential an agent presents on the write plane. The token
-// value itself is never stored — only Hash — so a database dump cannot be
-// replayed against the service.
-//
-// v1 ships one token plus mandatory provenance rather than per-agent scopes:
-// the refs rule (an agent credential can only move refs under BranchPrefix) is
-// the boundary that actually bounds the damage, and it holds with a single
-// shared token. Per-space scoping is a later column on this row plus a filter
-// clause, not an architectural change.
-type AgentToken struct {
- ID int
- Name string
- Hash []byte
- Created time.Time
- Revoked *time.Time
-}
-
-// Active reports whether the token may still authenticate.
-func (t *AgentToken) Active() bool { return t.Revoked == nil }
-
-// GenerateToken mints a fresh token value: TokenBytes of crypto/rand, URL-safe
-// base64 without padding. This is the only moment the plaintext exists; the
-// caller shows it to the operator once and stores only HashToken(it).
-func GenerateToken() (string, error) {
- b := make([]byte, TokenBytes)
- if _, err := rand.Read(b); err != nil {
- return "", fmt.Errorf("generate agent token: %w", err)
- }
- return base64.RawURLEncoding.EncodeToString(b), nil
-}
-
-// HashToken is the one-way function between a presented token and the stored
-// agent_token.token_hash. SHA-256 is the right tool here rather than a password
-// KDF: the input is 256 bits of uniform randomness we generated, not a
-// human-chosen secret, so there is no dictionary to stretch against.
-func HashToken(token string) []byte {
- sum := sha256.Sum256([]byte(token))
- return sum[:]
-}
-
-// TokenMatches compares a stored hash with the hash of a presented token in
-// constant time. Byte-wise early exit on a hash comparison leaks how many
-// leading bytes an attacker guessed right, which is enough to walk a forged
-// value into place one byte at a time; subtle.ConstantTimeCompare does not.
-func TokenMatches(stored []byte, token string) bool {
- return subtle.ConstantTimeCompare(stored, HashToken(token)) == 1
-}
-
-// CreateAgentToken stores a token by hash and returns the row. The plaintext is
-// never passed to this function and never reaches SQL — callers hash with
-// HashToken and keep the value only long enough to show it once.
-//
-// A token_hash UNIQUE violation means the same token was registered twice —
-// for 32 random bytes, that is a caller re-registering a value it already had,
-// not a collision — and is mapped to ErrTokenExists.
-func (s *Store) CreateAgentToken(ctx context.Context, name string, hash []byte) (*AgentToken, error) {
- if name == "" {
- return nil, fmt.Errorf("create agent token: name is required")
- }
- if len(hash) != sha256.Size {
- return nil, fmt.Errorf("create agent token: hash must be %d bytes, got %d",
- sha256.Size, len(hash))
- }
- const q = `
-INSERT INTO agent_token (name, token_hash, created)
-VALUES ($1, $2, $3)
-RETURNING id, created`
- t := AgentToken{Name: name, Hash: hash}
- err := s.q.QueryRowContext(ctx, q, name, hash, time.Now().UTC()).Scan(&t.ID, &t.Created)
- if err != nil {
- var pqErr *pq.Error
- if errors.As(err, &pqErr) && pqErr.Code == "23505" {
- return nil, ErrTokenExists
- }
- return nil, fmt.Errorf("create agent token %q: %w", name, err)
- }
- return &t, nil
-}
-
-// AgentTokenByHash looks a token up by its stored hash. It does not consider
-// revocation — use AuthenticateAgentToken for the authorization decision.
-// Returns ErrNotFound if no such token is registered.
-func (s *Store) AgentTokenByHash(ctx context.Context, hash []byte) (*AgentToken, error) {
- const q = `
-SELECT id, name, token_hash, created, revoked
-FROM agent_token
-WHERE token_hash = $1`
- var (
- t AgentToken
- revoked sql.NullTime
- )
- err := s.q.QueryRowContext(ctx, q, hash).Scan(&t.ID, &t.Name, &t.Hash, &t.Created, &revoked)
- if errors.Is(err, sql.ErrNoRows) {
- return nil, ErrNotFound
- }
- if err != nil {
- return nil, fmt.Errorf("agent token by hash: %w", err)
- }
- if revoked.Valid {
- r := revoked.Time
- t.Revoked = &r
- }
- return &t, nil
-}
-
-// AuthenticateAgentToken is the authorization boundary: it hashes the presented
-// token, looks the row up by that hash, re-verifies the stored hash against the
-// presentation in constant time, and rejects a revoked token.
-//
-// The re-verification is not redundant with the SQL equality. The index lookup
-// is what finds the row; TokenMatches is what decides, and it is the one
-// comparison an attacker can time. Returns ErrNotFound for an unknown token and
-// ErrTokenRevoked for a known but revoked one; both are 401 at the API edge.
-func (s *Store) AuthenticateAgentToken(ctx context.Context, token string) (*AgentToken, error) {
- if token == "" {
- return nil, ErrNotFound
- }
- t, err := s.AgentTokenByHash(ctx, HashToken(token))
- if err != nil {
- return nil, err
- }
- if !TokenMatches(t.Hash, token) {
- // The row was found by hash equality, so a mismatch here means the
- // stored hash is not what the index matched on — corruption, not a bad
- // credential.
- return nil, fmt.Errorf("agent token %d: stored hash does not verify", t.ID)
- }
- if !t.Active() {
- return nil, fmt.Errorf("%w: token %q revoked at %s", ErrTokenRevoked, t.Name, t.Revoked)
- }
- return t, nil
-}
-
-// RevokeAgentToken stamps a token revoked. Revocation is a stamp rather than a
-// delete so the audit trail keeps naming the token that made past proposals.
-// Revoking an already-revoked token is a no-op that returns nil; re-revoking is
-// not an error worth failing an operator over. Returns ErrNotFound if id does
-// not exist.
-func (s *Store) RevokeAgentToken(ctx context.Context, id int) error {
- const q = `UPDATE agent_token SET revoked = COALESCE(revoked, $2) WHERE id = $1`
- res, err := s.q.ExecContext(ctx, q, id, time.Now().UTC())
- if err != nil {
- return fmt.Errorf("revoke agent token %d: %w", id, err)
- }
- return requireOne(res, "revoke agent token")
-}
-
-// ListAgentTokens returns every token, newest first, so the operator can see
-// what exists and pick one to revoke. Hashes are included; there is nothing
-// secret about them and the reconciler-style tooling compares by them.
-func (s *Store) ListAgentTokens(ctx context.Context) ([]*AgentToken, error) {
- const q = `
-SELECT id, name, token_hash, created, revoked
-FROM agent_token
-ORDER BY created DESC, id DESC`
- rows, err := s.q.QueryContext(ctx, q)
- if err != nil {
- return nil, fmt.Errorf("list agent tokens: %w", err)
- }
- defer rows.Close()
- var out []*AgentToken
- for rows.Next() {
- var (
- t AgentToken
- revoked sql.NullTime
- )
- if err := rows.Scan(&t.ID, &t.Name, &t.Hash, &t.Created, &revoked); err != nil {
- return nil, fmt.Errorf("scan agent token: %w", err)
- }
- if revoked.Valid {
- r := revoked.Time
- t.Revoked = &r
- }
- out = append(out, &t)
- }
- if err := rows.Err(); err != nil {
- return nil, fmt.Errorf("iterate agent tokens: %w", err)
- }
- return out, nil
-}
D db/token_test.go => db/token_test.go +0 -118
@@ 1,118 0,0 @@
-package db
-
-import (
- "bytes"
- "context"
- "errors"
- "testing"
-)
-
-func TestAgentTokenLifecycle(t *testing.T) {
- s, pool, cleanup := newTestStore(t)
- defer cleanup()
- ctx := context.Background()
-
- token, err := GenerateToken()
- if err != nil {
- t.Fatalf("generate token: %v", err)
- }
- hash := HashToken(token)
-
- created, err := s.CreateAgentToken(ctx, "spec-writer", hash)
- if err != nil {
- t.Fatalf("create token: %v", err)
- }
- if created.ID == 0 || !created.Active() {
- t.Fatalf("unexpected created token: %+v", created)
- }
-
- // The plaintext must never have reached the database.
- var found int
- if err := pool.QueryRowContext(ctx,
- `SELECT count(*) FROM agent_token WHERE encode(token_hash, 'escape') LIKE '%' || $1 || '%'`,
- token).Scan(&found); err != nil {
- t.Fatalf("scan for plaintext: %v", err)
- }
- if found != 0 {
- t.Fatal("the token plaintext is recoverable from the row")
- }
-
- if _, err := s.CreateAgentToken(ctx, "duplicate", hash); !errors.Is(err, ErrTokenExists) {
- t.Fatalf("duplicate token = %v, want ErrTokenExists", err)
- }
-
- byHash, err := s.AgentTokenByHash(ctx, hash)
- if err != nil {
- t.Fatalf("lookup by hash: %v", err)
- }
- if byHash.ID != created.ID || byHash.Name != "spec-writer" || !bytes.Equal(byHash.Hash, hash) {
- t.Fatalf("unexpected lookup: %+v", byHash)
- }
- if _, err := s.AgentTokenByHash(ctx, HashToken("nope")); !errors.Is(err, ErrNotFound) {
- t.Fatalf("unknown hash = %v, want ErrNotFound", err)
- }
-
- auth, err := s.AuthenticateAgentToken(ctx, token)
- if err != nil {
- t.Fatalf("authenticate: %v", err)
- }
- if auth.ID != created.ID {
- t.Fatalf("authenticated the wrong token: %+v", auth)
- }
- if _, err := s.AuthenticateAgentToken(ctx, token+"x"); !errors.Is(err, ErrNotFound) {
- t.Fatalf("wrong token = %v, want ErrNotFound", err)
- }
- if _, err := s.AuthenticateAgentToken(ctx, ""); !errors.Is(err, ErrNotFound) {
- t.Fatalf("empty token = %v, want ErrNotFound", err)
- }
-
- // Revocation is a stamp, so the row stays for the audit trail.
- if err := s.RevokeAgentToken(ctx, created.ID); err != nil {
- t.Fatalf("revoke: %v", err)
- }
- if _, err := s.AuthenticateAgentToken(ctx, token); !errors.Is(err, ErrTokenRevoked) {
- t.Fatalf("revoked token = %v, want ErrTokenRevoked", err)
- }
- revoked, err := s.AgentTokenByHash(ctx, hash)
- if err != nil {
- t.Fatalf("lookup revoked: %v", err)
- }
- if revoked.Active() || revoked.Revoked == nil {
- t.Fatalf("revocation did not stick: %+v", revoked)
- }
- first := *revoked.Revoked
-
- // Re-revoking keeps the original timestamp rather than moving it.
- if err := s.RevokeAgentToken(ctx, created.ID); err != nil {
- t.Fatalf("re-revoke: %v", err)
- }
- again, err := s.AgentTokenByHash(ctx, hash)
- if err != nil {
- t.Fatalf("lookup revoked: %v", err)
- }
- if !again.Revoked.Equal(first) {
- t.Fatalf("re-revoking moved the timestamp: %v -> %v", first, *again.Revoked)
- }
-
- if err := s.RevokeAgentToken(ctx, 99999); !errors.Is(err, ErrNotFound) {
- t.Fatalf("revoking a missing token = %v, want ErrNotFound", err)
- }
-
- second, err := GenerateToken()
- if err != nil {
- t.Fatalf("generate token: %v", err)
- }
- if _, err := s.CreateAgentToken(ctx, "notes-writer", HashToken(second)); err != nil {
- t.Fatalf("create second token: %v", err)
- }
- list, err := s.ListAgentTokens(ctx)
- if err != nil {
- t.Fatalf("list tokens: %v", err)
- }
- if len(list) != 2 || list[0].Name != "notes-writer" {
- t.Fatalf("unexpected token list (newest first): %+v", list)
- }
- if _, err := s.AuthenticateAgentToken(ctx, second); err != nil {
- t.Fatalf("authenticate second token: %v", err)
- }
-}
M db/unit_test.go => db/unit_test.go +77 -57
@@ 2,7 2,6 @@ package db
import (
"context"
- "crypto/sha256"
"database/sql"
"errors"
"os"
@@ 17,9 16,9 @@ import (
)
// These tests need no database. They cover the logic that decides things —
-// transition legality, token comparison, batch duplicate detection — plus the
-// agreement between schema.sql and the migration, which is otherwise only
-// discovered on a fresh install.
+// transition legality, batch duplicate detection — plus the agreement between
+// schema.sql and the migrations, which is otherwise only discovered on a fresh
+// install.
func TestProposalBranch(t *testing.T) {
for _, tc := range []struct {
@@ 67,48 66,6 @@ func TestProposalBranch(t *testing.T) {
}
}
-func TestHashTokenAndMatches(t *testing.T) {
- tok, err := GenerateToken()
- if err != nil {
- t.Fatalf("generate token: %v", err)
- }
- if len(tok) < 40 {
- t.Fatalf("token %q is implausibly short for %d bytes of entropy", tok, TokenBytes)
- }
- hash := HashToken(tok)
- if len(hash) != sha256.Size {
- t.Fatalf("HashToken returned %d bytes, want %d", len(hash), sha256.Size)
- }
- if strings.Contains(string(hash), tok) {
- t.Fatal("hash must not contain the token")
- }
- if !TokenMatches(hash, tok) {
- t.Fatal("TokenMatches rejected the token it hashed")
- }
- if TokenMatches(hash, tok+"x") {
- t.Fatal("TokenMatches accepted a different token")
- }
- if TokenMatches(hash, "") {
- t.Fatal("TokenMatches accepted the empty token")
- }
- if TokenMatches(nil, tok) {
- t.Fatal("TokenMatches accepted a nil stored hash")
- }
- if TokenMatches(hash[:16], tok) {
- t.Fatal("TokenMatches accepted a truncated stored hash")
- }
-
- // Two mints must differ; a repeated value would mean the generator is not
- // actually random and every token would be the same credential.
- other, err := GenerateToken()
- if err != nil {
- t.Fatalf("generate token: %v", err)
- }
- if other == tok {
- t.Fatal("GenerateToken returned the same value twice")
- }
-}
-
func TestDuplicateDocIDs(t *testing.T) {
ref := func(id, path string) DocRef {
return DocRef{ID: docID(t, id), Path: path}
@@ 236,15 193,6 @@ func TestResolveArgumentsValidated(t *testing.T) {
}); !errors.Is(err, core.ErrReservedName) {
t.Fatalf("CreateProject of the meta-project = %v, want ErrReservedName", err)
}
- if _, err := s.CreateAgentToken(ctx, "", HashToken("x")); err == nil {
- t.Fatal("CreateAgentToken without a name must fail")
- }
- if _, err := s.CreateAgentToken(ctx, "ci", []byte("short")); err == nil {
- t.Fatal("CreateAgentToken with a non-sha256 hash must fail")
- }
- if _, err := s.AuthenticateAgentToken(ctx, ""); !errors.Is(err, ErrNotFound) {
- t.Fatalf("authenticating an empty token = %v, want ErrNotFound", err)
- }
if err := s.SetDigestMark(ctx, "", time.Now()); err == nil {
t.Fatal("SetDigestMark without an owner must fail")
}
@@ 344,7 292,7 @@ func TestSchemaMatchesMigration(t *testing.T) {
}
fromSchema := normalizeStatements(string(schema))
- fromMigration := ups
+ fromMigration := applyDrops(t, ups)
if len(fromSchema) != len(fromMigration) {
t.Fatalf("schema.sql has %d statements, migration Up has %d:\n%v\n%v",
len(fromSchema), len(fromMigration), fromSchema, fromMigration)
@@ 372,14 320,86 @@ func TestSchemaMatchesMigration(t *testing.T) {
// a design change rather than a quiet migration. Phase 5b is that design
// change: the anchoring model was settled against the built review UI, so
// the table is now required present like any other.
- for _, want := range []string{"space", "document_id", "proposal", "agent_token",
+ for _, want := range []string{"space", "document_id", "proposal",
"index_stamp", "digest_mark", "project", "project_space", "comment"} {
if !contains(created, want) {
t.Errorf("table %q is missing from schema.sql", want)
}
}
+
+ // agent_token is required *absent*: agent credentials are issued by
+ // tokens.sr.ht and validated by signature, so a service that still had the
+ // table would still have a second door into the write plane.
+ if contains(created, "agent_token") {
+ t.Error("agent_token is back in schema.sql; agent credentials come from tokens.sr.ht")
+ }
}
+// applyDrops folds a migration history the way Postgres does: a DROP TABLE
+// removes the CREATE TABLE it names, the indexes on it, and itself.
+//
+// Without the fold the comparison above could only hold for an append-only
+// history, and the first migration to remove a table — agent_token, when agent
+// issuance moved to tokens.sr.ht — would have had to weaken the check instead of
+// being checked by it. A statement that still names a dropped table after the
+// fold is a fatal error rather than a silent pass: it means the history does
+// something (an ALTER, a backfill) that this small folder does not model, and
+// guessing would make the agreement test lie.
+func applyDrops(t *testing.T, stmts []string) []string {
+ t.Helper()
+ var out []string
+ for _, stmt := range stmts {
+ name, dropped := droppedTable(stmt)
+ if !dropped {
+ out = append(out, stmt)
+ continue
+ }
+ kept := out[:0]
+ for _, prev := range out {
+ if createsTable(prev, name) || indexesTable(prev, name) {
+ continue
+ }
+ if mentions(prev, name) {
+ t.Fatalf("migration drops %q, but this earlier statement still names it "+
+ "and is not a CREATE TABLE or CREATE INDEX:\n %s", name, prev)
+ }
+ kept = append(kept, prev)
+ }
+ out = kept
+ }
+ return out
+}
+
+var (
+ dropTableRe = regexp.MustCompile(`^DROP TABLE (?:IF EXISTS )?"?(\w+)"?$`)
+ createIndex = regexp.MustCompile(`^CREATE (?:UNIQUE )?INDEX \w+ ON "?(\w+)"?[ (]`)
+ identifierRe = func(name string) *regexp.Regexp { return regexp.MustCompile(`\b` + name + `\b`) }
+)
+
+func droppedTable(stmt string) (string, bool) {
+ m := dropTableRe.FindStringSubmatch(strings.TrimSpace(stmt))
+ if m == nil {
+ return "", false
+ }
+ return m[1], true
+}
+
+func createsTable(stmt, name string) bool {
+ rest, ok := strings.CutPrefix(stmt, "CREATE TABLE ")
+ if !ok {
+ return false
+ }
+ return strings.HasPrefix(rest, name+"(") || strings.HasPrefix(rest, name+" ") ||
+ strings.HasPrefix(rest, `"`+name+`"`)
+}
+
+func indexesTable(stmt, name string) bool {
+ m := createIndex.FindStringSubmatch(stmt)
+ return m != nil && m[1] == name
+}
+
+func mentions(stmt, name string) bool { return identifierRe(name).MatchString(stmt) }
+
func splitBrant(src string) (up, down string, ok bool) {
i := strings.Index(src, "-- +brant Up")
j := strings.Index(src, "-- +brant Down")
M docs/DESIGN.md => docs/DESIGN.md +82 -30
@@ 56,7 56,7 @@ modification**. Runs at `https://spec.srht.bigb.es`.
| Read contract | **Pinned `?rev=` plus an approved/draft split.** Nearly free, since the review UI needs blob→render at arbitrary revs anyway, and it makes `X-Agent-Base` provenance auditable rather than decorative. |
| Storage tiers | **One.** Git objects are read directly; the materialized checkout is dropped, leaving a bleve index and a blob-sha-keyed render cache as the only caches. |
| Volume | **Tens of documents a day or fewer.** warren's batch index rebuild is absorbed unchanged; incremental indexing is explicitly not built. |
-| Agent tokens | **One token plus mandatory provenance.** The refs rule (agents write only `proposals/*`) is the boundary that matters; per-space scoping deferred. |
+| Agent tokens | **Issued by tokens.sr.ht, plus mandatory provenance.** v1 shipped one shared `agent_token` row; that plane is gone — see "Where agent credentials come from". The refs rule (agents write only `proposals/*`) is still the boundary that matters; per-space scoping deferred. |
| Push validation | **Your pushes are validated too**, with a `--push-option=skip-validation` escape hatch. |
| Name | **`spec.sr.ht`** at `spec.srht.bigb.es`; module `sourcecraft.dev/bigbes/sr-ht-spec`, binary `specsrht`. |
@@ 72,8 72,8 @@ One Go module **`sourcecraft.dev/bigbes/sr-ht-spec`**, one binary `specsrht`
│ │ update + post-receive hooks
├──► one global bleve index (cache)
├──► render cache, keyed by blob sha (cache)
- └──► Postgres (proposals, comments, agent
- tokens, ID registry)
+ └──► Postgres (proposals, comments,
+ ID registry)
you ──browser──────────► read + review UI (no editing)
```
@@ 821,27 821,70 @@ approver lists, approval counts, request-changes round-trips, and per-human ACL
rows. The unified-login cookie is still needed — not to tell users apart, but to
tell *you* from an unauthenticated request.
-**v1 ships one agent token plus mandatory provenance, not per-agent scopes.**
-The distinction worth drawing is between the two boundaries this design has:
+**v1 shipped one shared agent token plus mandatory provenance, not per-agent
+scopes.** The distinction worth drawing is between the two boundaries this
+design has:
- **The refs rule is the boundary that matters, and it is free.** No agent
credential can move the approved branch — only `proposals/*`. That is what
- actually bounds the damage a confused or runaway agent can do, and it holds
- with a single shared token.
+ actually bounds the damage a confused or runaway agent can do, and it held
+ even with a single shared token.
- **Per-space scoping is the boundary that can wait.** Preventing a
notes-writing agent from proposing into `specs/` is real defence in depth, but
every agent here is a session you launched yourself, and a bad proposal is
- visible and rejectable rather than destructive. Adding scopes later is a column
- on the token row plus a filter clause — not an architectural change.
+ visible and rejectable rather than destructive. Adding scopes later is a
+ filter clause on top of the credential — not an architectural change.
What is **not** optional is provenance: agent identity and session ID are
-required on every write and recorded in git trailers. One token still yields a
-full audit trail, because the trailers, not the credential, are what identify
-who did what.
+required on every write and recorded in git trailers. The credential is not what
+identifies who did what — the trailers are — which is why one shared token still
+yielded a full audit trail, and why per-agent credentials do not replace them.
+
+### Where agent credentials come from
+
+**Issuance is tokens.sr.ht's, not ours.** The `agent_token` table is gone: no
+mint, no list, no revoke, no `specsrht token`, no row to look a presented secret
+up in. An agent authenticates with a tokens.sr.ht *working token* — signed by the
+instance, expiring, owned by a meta.sr.ht account, carrying a grant set — which
+`authn/` verifies locally through sr-ht-ecore's `bearer` package. See
+`sourcehut-tokens/SPEC.md`, chapters 2, 3 and 6.
+
+What that changes, and what it deliberately does not:
+
+- **Two vocabulary entries, declared here and not by the daemon:**
+ `spec:propose` for the write plane and `spec:read` for every read surface (web,
+ `/query`, MCP — checked per tool there, because one endpoint carries both
+ kinds). An unknown grant admits nobody.
+- **The refs rule is untouched and is not replaced by grants.** A token carrying
+ `*` is still an agent to the receive path, and an agent still writes only
+ `proposals/*`. A grant says what a credential was minted for; the refs rule
+ says where a ref may point.
+- **Provenance is untouched.** `X-Agent` and `X-Agent-Session` are still
+ mandatory on every agent write, on the credential plane that remains exactly as
+ they were on the one that is gone.
+- **Every surface, including `git push`.** The SSH path used to check
+ `agent_token` directly, which is how it ended up being the one surface that
+ could not take an instance token; it goes through the same `authn.Resolver` now
+ and demands `spec:propose`, because a push by an agent is a proposal by another
+ transport.
+- **A working token whose owner is not `[sr.ht] owner-name` is refused (403),**
+ not admitted as a second identity: this instance answers to one human, and
+ `Principal.Owner` is read by the provenance committer, the refs rule and the
+ coreauth bridge, all written for exactly one.
+- **A `[tokens.sr.ht]` section is now required in config.ini.** With no issuer
+ there is no credential the service can check, so the daemon fails startup
+ rather than serving reads and refusing every agent write one request at a time.
+- **`/tokens` redirects to tokens.sr.ht** (SPEC ch. 7): the services' own token
+ pages become links to the one that issues.
+
+**Deploy gate.** Removing the local plane locks out every agent still configured
+with the shared secret — over HTTP and over `git push` — the moment it is
+deployed. Every agent must hold a tokens.sr.ht working token *before* the
+migration that drops the table runs.
### Provenance
-One agent token (no roles, no per-space scopes — see above) plus a **required
+An agent credential (no roles, no per-space scopes — see above) plus a **required
agent identity string**. Every commit records it in a way that survives clone:
```
@@ 968,6 1011,7 @@ convention already used by both siblings. `cache` is ours.
| `[sr.ht] internal-ipnet` | this host must fall inside it or internal GraphQL calls are rejected |
| `[webhooks] private-key` | **`crypto.InitCrypto` fatally requires it even though v1 emits no webhooks** |
| `[meta.sr.ht] origin` | login/logout redirects, profile fetch, PAT validation |
+| `[tokens.sr.ht] origin` | **required**: the only issuer of agent credentials. The internal form is the revocation check's endpoint; the external one is where `/tokens` sends a browser. The daemon refuses to start without it |
| `[git.sr.ht] repos` / `api-origin` | only if read-only mounts of `docs/` dirs in git.sr.ht repos are ever enabled (not v1) |
### Wiring checklist
@@ 985,6 1029,11 @@ convention already used by both siblings. `cache` is ours.
4. **internal-ipnet** — same prerequisite as both siblings.
5. **Migrations** — `specsrht-migrate`, a brant wrapper, copied from
`doltsrht-migrate`.
+6. **Agent credentials before migration 0005.** `0005_drop_agent_token` removes
+ the local credential plane, and with `migrate-on-upgrade=yes` it runs on
+ deploy. Every agent — including any that pushes over SSH — must already hold a
+ tokens.sr.ht working token with `spec:propose` (and `spec:read` to read).
+ Deploying first locks all of them out at once.
### GraphQL: a read schema at our own `/query` in Phase 2
@@ 1078,15 1127,16 @@ core/ pure domain. Space/doc/rev/ID validation, .spec.yml policy,
proposal state machine, sentinel errors. No external deps.
gitx/ bare-repo lifecycle, tree walk, blob read, proposal branches,
the tree-splice merge, refs-rule enforcement. go-git only.
-db/ Postgres: proposals, ID registry, agent token, index stamps.
+db/ Postgres: proposals, ID registry, index stamps. No credential
+ table — agent tokens are issued and signed by tokens.sr.ht.
doc/ absorbed warren vault/ + render/: frontmatter, Archive,
goldmark rendering, wikilink resolution. Fed by gitx trees.
search/ absorbed warren index/ + search/: one global bleve index,
query-time project filtering.
prosediff/ word-level prose diff over rendered block structure. Net-new,
riskiest, and the Phase 0 gate.
-authn/ unified-login cookie -> identity, agent token validation,
- provenance trailer construction.
+authn/ unified-login cookie -> identity, tokens.sr.ht working-token
+ validation (grants included), provenance trailer construction.
service/ orchestration: read, propose, merge, reconcile, digest. The
single layer REST, MCP and GraphQL all call.
api/ REST handlers (write plane + JSON reads).
@@ 1147,13 1197,10 @@ CREATE TABLE proposal (
);
CREATE INDEX ON proposal (state, created DESC);
-CREATE TABLE agent_token (
- id SERIAL PRIMARY KEY,
- name TEXT NOT NULL,
- token_hash BYTEA NOT NULL UNIQUE,
- created TIMESTAMPTZ NOT NULL DEFAULT now(),
- revoked TIMESTAMPTZ
-);
+-- There is no credential table. `agent_token` (id, name, token_hash, created,
+-- revoked) stood here until agent issuance moved to tokens.sr.ht; migration
+-- 0005 drops it. A working token is signed rather than stored, so there is
+-- nothing to look up.
-- Index staleness: compared against the space's approved head.
CREATE TABLE index_stamp (
@@ 1270,7 1317,9 @@ importing only the committed `core/`.
migration.
- `authn/` — cookie decrypt to identity, agent-token validation, provenance
trailer construction. Tests forge cookies with a synthesized ini (random fernet
- key + ed25519 seed), as dolt.sr.ht's `authn/` does.
+ key + ed25519 seed), as dolt.sr.ht's `authn/` does. (Phase 1 validated spec's
+ own `agent_token`; that plane is gone — see "Where agent credentials come
+ from".)
**Wave B (serial).** `service/` read + reconcile paths, `hooks/` (the `update`
and `post-receive` shims and their daemon RPC endpoints), `cmd/specsrht` skeleton,
@@ 1332,8 1381,10 @@ degrading, which is the point of listing them.
hook with a message naming the collision; retry with
`--push-option=skip-validation` → accepted.
4. `git push --force` to the approved branch → rejected.
-5. An agent token proposing to `proposals/*` → accepted; the same token
- attempting the approved branch → rejected.
+5. An agent token carrying `spec:propose` proposing to `proposals/*` → accepted;
+ the same token attempting the approved branch → rejected. A token without
+ `spec:propose` → 403 on both the HTTP write plane and `git push`; a token from
+ the removed `agent_token` plane → 401 everywhere.
6. Propose via MCP → response carries a URL; opening it shows the prose diff;
approve → merges, and the document's approved text changes.
7. Propose against a stale base → 409 with the current head.
@@ 1388,9 1439,10 @@ degrading, which is the point of listing them.
not faked: an invented href would be worse than an honest missing link. The
cheaper answer may be to **prefer Mermaid in fenced blocks by convention** —
it stays text, diffs properly, and needs none of the above.
-- **Agent token distribution.** How a Claude Code session actually acquires a
- scoped token — long-lived value in the environment, or minted per session.
- Per-session is better for provenance and revocation but needs an issuing flow.
+- ~~**Agent token distribution.**~~ **Answered by tokens.sr.ht.** A session holds
+ a long-lived parent token in its environment and exchanges it for a short
+ working token (`POST /exchange`); per-session credentials no longer need an
+ issuing flow of ours, because the issuing flow is the daemon's.
- **Retention for the firehose half.** Auto-merged notes accumulate forever by
default. Whether they expire, get compacted, or are simply never deleted
affects repo growth and index size, and is easier to decide now than later.
@@ 1407,7 1459,7 @@ into the sections above:
| `?rev=` pinning and approved/draft split? | **Both.** Nearly free — the review UI needs blob→render at arbitrary revs regardless — and it makes `X-Agent-Base` auditable. |
| Materialized checkout? | **Dropped.** One read path over git objects; render cache keyed by blob sha. |
| Firehose volume? | **Tens of documents a day or fewer.** Batch index rebuild absorbed as-is; incremental indexing not built; retention a non-issue. |
-| Agent token machinery? | **One token + mandatory provenance.** The refs rule is the boundary that matters; per-space scoping deferred to a column and a filter clause. |
+| Agent token machinery? | **One token + mandatory provenance** (2026-07-22). Superseded: issuance moved to tokens.sr.ht and the local plane was removed, so a credential is now per-agent, owned and grant-carrying. The refs rule is still the boundary that matters; per-space scoping still deferred to a filter clause. |
| Validate your own pushes? | **Yes, with `--push-option=skip-validation`.** Guards typos that corrupt the global ID registry, without a lockout risk. |
| Import external corpora? | **Probably eventually; left unspecified.** Global IDs are the only forward compatibility required. |
| Name? | **`spec.sr.ht` at `spec.srht.bigb.es`**, module `sourcecraft.dev/bigbes/sr-ht-spec`, binary `specsrht`. |
M graph/grant_test.go => graph/grant_test.go +5 -2
@@ 37,8 37,11 @@ func TestGateChecksIdentityThenGrant(t *testing.T) {
{"anonymous", authn.Anonymous(), http.StatusUnauthorized, false},
{"owner cookie", authn.Principal{Kind: authn.KindOwner, Owner: "bigbes"}, http.StatusOK, true},
{
- "local agent token",
- authn.Principal{Kind: authn.KindAgent, Owner: "bigbes", Plane: authn.PlaneLocal},
+ // An agent a local process asserted: no credential, so no grant set
+ // to read. The resolver never produces one, but the gate must not
+ // invent a refusal for a principal that carries none.
+ "locally asserted agent",
+ authn.Principal{Kind: authn.KindAgent, Owner: "bigbes"},
http.StatusOK, true,
},
{
M graph/graph_test.go => graph/graph_test.go +43 -21
@@ 19,7 19,9 @@ import (
"github.com/fernet/fernet-go"
"github.com/go-chi/chi/v5"
"github.com/vaughan0/go-ini"
+ "sourcecraft.dev/bigbes/sr-ht-core/auth"
"sourcecraft.dev/bigbes/sr-ht-core/crypto"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/bearer"
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
@@ 56,7 58,6 @@ const (
headRev = "1111111111111111111111111111111111111111"
oldRev = "2222222222222222222222222222222222222222"
absentRev = "3333333333333333333333333333333333333333"
- agentTk = "test-agent-token"
)
var (
@@ 289,15 290,45 @@ func (f *fakeProposals) ListProposals(_ context.Context, space core.SpaceRef, st
return out, nil
}
-// stubTokenStore knows exactly one live agent token.
-type stubTokenStore struct{}
+// stubUsers resolves the owner an instance token names to a local row.
+type stubUsers struct{}
-func (stubTokenStore) LookupAgentToken(_ context.Context, hash []byte) (authn.AgentToken, error) {
- want := authn.HashToken(agentTk)
- if string(hash) != string(want) {
- return authn.AgentToken{}, authn.ErrUnknownToken
+func (stubUsers) LookupUser(_ context.Context, username string) (authn.InstanceUser, error) {
+ return authn.InstanceUser{ID: 1, Username: username}, nil
+}
+
+// testResolver is the resolver the daemon builds, with the one agent credential
+// plane wired. Its origin is never reached: these tests present stateless
+// tokens, which carry no row id and so skip the revocation round trip entirely.
+func testResolver(t *testing.T) *authn.Resolver {
+ t.Helper()
+ v, err := bearer.New(bearer.Options{
+ Origin: "https://tokens.srht.invalid",
+ ClientID: "spec.sr.ht",
+ NodeID: "graph-test",
+ })
+ if err != nil {
+ t.Fatalf("bearer.New: %v", err)
}
- return authn.AgentToken{ID: 1, Name: "test", Hash: want}, nil
+ resolver, err := authn.NewResolver("bigbes", authn.WithInstancePlane(v, stubUsers{}))
+ if err != nil {
+ t.Fatalf("NewResolver: %v", err)
+ }
+ return resolver
+}
+
+// agentToken mints a signed tokens.sr.ht working token carrying grantString. It
+// cannot be a package-level constant the way the old opaque secret was: the
+// signing key is established by TestMain.
+func agentToken(grantString string) string {
+ bt := &auth.BearerToken{
+ Version: auth.TokenVersion,
+ Expires: auth.ToTimestamp(time.Now().Add(time.Hour)),
+ Grants: grantString,
+ ClientID: bearer.TokensClientID,
+ Username: "bigbes",
+ }
+ return bt.Encode()
}
// ---- harness --------------------------------------------------------------
@@ 310,10 341,7 @@ type harness struct {
func newHarness(t *testing.T, withProposals bool) harness {
t.Helper()
- resolver, err := authn.NewResolver("bigbes", stubTokenStore{})
- if err != nil {
- t.Fatalf("NewResolver: %v", err)
- }
+ resolver := testResolver(t)
searcher := &fakeSearcher{}
opts := Options{Reader: newFakeReader(), Searcher: searcher, Resolver: resolver}
var proposals *fakeProposals
@@ 645,7 673,7 @@ func TestIntrospectionIsGated(t *testing.T) {
func TestAgentTokenReads(t *testing.T) {
h := newHarness(t, false)
r := post(t, h, `{ document(space: "~bigbes/rfcs", id: "SPEC-0007") { title } }`, func(req *http.Request) {
- req.Header.Set("Authorization", "Bearer "+agentTk)
+ req.Header.Set("Authorization", "Bearer "+agentToken("spec:read"))
})
var got struct{ Document struct{ Title string } }
ok(t, r, &got)
@@ 1050,10 1078,7 @@ func TestSchemaMutationsAreWebhooksOnly(t *testing.T) {
// middleware of its own, the endpoint at /query, and a query that goes through.
// The daemon's one line is the line under test here.
func TestMountedOnAChiRouter(t *testing.T) {
- resolver, err := authn.NewResolver("bigbes", stubTokenStore{})
- if err != nil {
- t.Fatalf("NewResolver: %v", err)
- }
+ resolver := testResolver(t)
srv, err := New(Options{Reader: newFakeReader(), Searcher: &fakeSearcher{}, Resolver: resolver})
if err != nil {
t.Fatalf("New: %v", err)
@@ 1077,10 1102,7 @@ func TestMountedOnAChiRouter(t *testing.T) {
// New refuses a half-wired server at startup rather than failing inside the
// first query.
func TestNewRequiresItsSeams(t *testing.T) {
- resolver, err := authn.NewResolver("bigbes", stubTokenStore{})
- if err != nil {
- t.Fatalf("NewResolver: %v", err)
- }
+ resolver := testResolver(t)
cases := map[string]Options{
"no reader": {Searcher: &fakeSearcher{}, Resolver: resolver},
"no searcher": {Reader: newFakeReader(), Resolver: resolver},
M hooks/doc.go => hooks/doc.go +6 -3
@@ 76,12 76,15 @@
// daemon resolves it:
//
// SPECSRHT_PRINCIPAL "owner" or "agent" (required)
-// SPECSRHT_AGENT_TOKEN agent secret (required when kind=agent)
+// SPECSRHT_AGENT_TOKEN agent credential (required when kind=agent)
// SPECSRHT_AGENT agent identity string (provenance, optional)
// SPECSRHT_AGENT_SESSION agent session id (provenance, optional)
//
-// An agent's token is validated against the database on every push, so an
-// agent credential asserts nothing by itself. `owner` is different: it is an
+// The agent credential is a tokens.sr.ht working token — the same one the REST
+// and MCP planes take — validated on every push through the same authn.Resolver
+// those surfaces authenticate with, so an agent credential asserts nothing by
+// itself. It must carry the spec:propose grant: a push by an agent is a proposal
+// by another transport. `owner` is different: it is an
// assertion, trusted because sshd already authenticated the SSH key and the
// forced-command wrapper — which owns this environment — is what sets it.
// That wrapper is therefore part of the trust boundary: sshd must not
M hooks/e2e_test.go => hooks/e2e_test.go +87 -1
@@ 8,9 8,13 @@ import (
"path/filepath"
"strings"
"testing"
+ "testing/fstest"
"github.com/go-git/go-git/v5/plumbing"
+ "sourcecraft.dev/bigbes/sr-ht-core/config"
+ "sourcecraft.dev/bigbes/sr-ht-core/crypto"
+
"sourcecraft.dev/bigbes/sr-ht-spec/core"
"sourcecraft.dev/bigbes/sr-ht-spec/gitx"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
@@ 23,10 27,26 @@ import (
// the test binary is enough to make a real `git push` run this package's code
// through git's real receive path. That is the production dispatch mechanism,
// unchanged and unmocked — not a stand-in for it.
+//
+// It also initialises crypto, because the agent credential this package
+// validates is a signed working token and the signing key lives in core-go's
+// globals; the keys are the ones core-go's own tests use. A hook process skips
+// that: a hook reads no config and validates nothing, which is the property
+// that lets it run with no state at all.
func TestMain(m *testing.M) {
if _, _, isHook := ModeFromArgs(os.Args); isHook {
os.Exit(Run(Runtime{Args: os.Args}))
}
+ config.FS = fstest.MapFS{
+ "config.ini": &fstest.MapFile{Data: []byte(`
+[webhooks]
+private-key=ebzsjPaN6E13ln/FeNWly1C92q6bVMVdOnDo1HPl5fc=
+
+[sr.ht]
+network-key=tbuG-7Vh44vrDq1L_HKWkHnWrDOtJhEkPKPiauaLeuk=
+`)},
+ }
+ crypto.InitCrypto(config.LoadConfig())
os.Exit(m.Run())
}
@@ 158,7 178,7 @@ func newE2E(t *testing.T) *e2e {
t.Fatalf("InstallSpace: %v", err)
}
- back := newFakeBackend(t, root, nil)
+ back := newFakeBackend(t, root)
back.validate = realish(t, root)
srv, landed := startServer(t, back)
@@ 194,6 214,20 @@ func (e *e2e) push(t *testing.T, args ...string) (string, bool) {
return out, err == nil
}
+// pushAsAgent is the same push with the environment the forced-command wrapper
+// exports for an agent: a tokens.sr.ht working token and the provenance fields.
+func (e *e2e) pushAsAgent(t *testing.T, token string, args ...string) (string, bool) {
+ t.Helper()
+ env := map[string]string{
+ EnvPrincipal: string(PrincipalAgent),
+ EnvAgentToken: token,
+ EnvAgent: "claude-code/spec-writer",
+ EnvAgentSession: "8fb9c9a4-b078-4af1-89eb-d97c522f9921",
+ }
+ out, err := runGit(t, e.work, env, append([]string{"push", e.repo}, args...)...)
+ return out, err == nil
+}
+
const goodDoc = `---
id: SPEC-0001
title: Storage
@@ 299,6 333,58 @@ func TestEndToEndPush(t *testing.T) {
})
}
+// TestEndToEndAgentPush is the credential change under a real `git push`: an
+// agent holding a tokens.sr.ht working token can push, the refs rule still
+// confines it to proposals/*, and a token without spec:propose gets nowhere.
+//
+// Before this, the SSH path checked agent_token directly and would have refused
+// every one of these; it is now the same authn.Resolver the HTTP surfaces use.
+func TestEndToEndAgentPush(t *testing.T) {
+ e := newE2E(t)
+ e.write(t, "specs/0001-storage.md", goodDoc)
+ e.commit(t, "add the storage spec")
+
+ t.Run("a token without spec:propose cannot push", func(t *testing.T) {
+ out, ok := e.pushAsAgent(t, instanceToken("spec:read"), "main:refs/heads/proposals/1")
+ if ok {
+ t.Fatalf("a token with no propose grant was accepted:\n%s", out)
+ }
+ mentions(t, "the rejection", out, "spec.sr.ht rejected this push", "spec:propose")
+ if out, err := runGit(t, e.repo, nil, "rev-parse", "--verify", "refs/heads/proposals/1"); err == nil {
+ t.Errorf("the ref moved despite the refusal: %s", out)
+ }
+ })
+
+ t.Run("the old opaque agent token cannot push", func(t *testing.T) {
+ out, ok := e.pushAsAgent(t, "s3cret-from-agent-token", "main:refs/heads/proposals/1")
+ if ok {
+ t.Fatalf("a credential from the removed plane was accepted:\n%s", out)
+ }
+ mentionsNot(t, "the rejection", out, "s3cret-from-agent-token")
+ })
+
+ t.Run("an instance token with spec:propose pushes a proposal branch", func(t *testing.T) {
+ out, ok := e.pushAsAgent(t, instanceToken("spec:propose"), "main:refs/heads/proposals/1")
+ if !ok {
+ t.Fatalf("an agent holding a tokens.sr.ht token was refused:\n%s", out)
+ }
+ if head := strings.TrimSpace(gitMust(t, e.repo, "rev-parse", "refs/heads/proposals/1")); head == "" {
+ t.Error("the proposal branch was not created")
+ }
+ })
+
+ t.Run("the refs rule still confines it to the proposal prefix", func(t *testing.T) {
+ out, ok := e.pushAsAgent(t, instanceToken("*"), "main:refs/heads/main")
+ if ok {
+ t.Fatalf("an agent moved the approved branch:\n%s", out)
+ }
+ mentions(t, "the rejection", out, "spec.sr.ht rejected this push")
+ if out, err := runGit(t, e.repo, nil, "rev-parse", "--verify", "refs/heads/main"); err == nil {
+ t.Errorf("the approved branch moved despite the refusal: %s", out)
+ }
+ })
+}
+
// TestEndToEndFailsClosed is the fail-closed rule under a real push: with the
// daemon gone, the push is refused rather than silently accepted unvalidated.
func TestEndToEndFailsClosed(t *testing.T) {
M hooks/fixture_test.go => hooks/fixture_test.go +65 -29
@@ 6,15 6,19 @@ import (
"io"
"log/slog"
"net"
+ "net/http"
+ "net/http/httptest"
"os"
"path/filepath"
"strings"
"testing"
"time"
+ "sourcecraft.dev/bigbes/sr-ht-core/auth"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/bearer"
+
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
- "sourcecraft.dev/bigbes/sr-ht-spec/db"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
)
@@ 22,25 26,49 @@ const testOwner = "bigbes"
var testSpace = core.SpaceRef{Owner: testOwner, Name: "rfcs"}
-// fakeLookup is service.AgentTokenLookup over a map, so the agent path can be
-// exercised without Postgres. *service.TokenStore is what turns db/'s
-// ErrNotFound into authn's ErrUnknownToken, and that mapping is part of what
-// the server relies on, so the real adapter is used over this fake rather than
-// a fake authn.TokenStore.
-type fakeLookup struct {
- byHash map[string]*db.AgentToken
- err error
-}
+// instanceToken mints a signed tokens.sr.ht working token the way the daemon
+// does. It is a real credential checked by the real sr-ht-ecore validator,
+// because the point of routing the push path through authn.Resolver is that it
+// runs the identical check the HTTP surfaces run; a stub validator here would
+// only assert that the wiring calls something.
+func instanceToken(grantString string) string { return tokenFor(testOwner, grantString) }
-func (f *fakeLookup) AgentTokenByHash(_ context.Context, hash []byte) (*db.AgentToken, error) {
- if f.err != nil {
- return nil, f.err
+// foreignToken is a working token belonging to somebody who is not the instance
+// owner: valid, and refused all the same.
+func foreignToken(grantString string) string { return tokenFor("someone", grantString) }
+
+func tokenFor(username, grantString string) string {
+ bt := &auth.BearerToken{
+ Version: auth.TokenVersion,
+ Expires: auth.ToTimestamp(time.Now().Add(time.Hour)),
+ Grants: grantString,
+ ClientID: bearer.TokensClientID,
+ Username: username,
}
- tok, ok := f.byHash[string(hash)]
- if !ok {
- return nil, db.ErrNotFound
+ return bt.Encode()
+}
+
+// stubUsers resolves the owner an instance token names to a local row.
+type stubUsers struct{}
+
+func (stubUsers) LookupUser(_ context.Context, username string) (authn.InstanceUser, error) {
+ return authn.InstanceUser{ID: 1, Username: username}, nil
+}
+
+// fakeDaemonOrigin starts a stand-in for tokens.sr.ht's revocation endpoint —
+// 204 is live, 404 is revoked — and returns its origin. A dead one, with
+// nothing answering, is what a restarting daemon looks like from here.
+func fakeDaemonOrigin(t *testing.T, status int, dead bool) string {
+ t.Helper()
+ srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+ w.WriteHeader(status)
+ }))
+ if dead {
+ srv.Close()
+ return srv.URL
}
- return tok, nil
+ t.Cleanup(srv.Close)
+ return srv.URL
}
// fakeBackend is a Backend that answers from a script instead of a database.
@@ 49,7 77,6 @@ func (f *fakeLookup) AgentTokenByHash(_ context.Context, hash []byte) (*db.Agent
type fakeBackend struct {
root string
resolver *authn.Resolver
- tokens *service.TokenStore
// validate is the scripted answer, and the recorder: every request reaches
// it. A nil validate accepts everything.
@@ 58,23 85,32 @@ type fakeBackend struct {
seen []service.PushRequest
}
-func newFakeBackend(t *testing.T, root string, tokens map[string]*db.AgentToken) *fakeBackend {
+// newFakeBackend builds a backend whose resolver carries the real tokens.sr.ht
+// validator, pointed at a live fake daemon.
+func newFakeBackend(t *testing.T, root string) *fakeBackend {
t.Helper()
- lookup := &fakeLookup{byHash: map[string]*db.AgentToken{}}
- for secret, row := range tokens {
- lookup.byHash[string(authn.HashToken(secret))] = row
+ return newFakeBackendAgainst(t, root, fakeDaemonOrigin(t, http.StatusNoContent, false))
+}
+
+func newFakeBackendAgainst(t *testing.T, root, origin string) *fakeBackend {
+ t.Helper()
+ v, err := bearer.New(bearer.Options{
+ Origin: origin,
+ ClientID: service.ConfigSection,
+ NodeID: "hooks-test",
+ })
+ if err != nil {
+ t.Fatalf("bearer.New: %v", err)
}
- store := service.NewTokenStore(lookup)
- resolver, err := authn.NewResolver(testOwner, store)
+ resolver, err := authn.NewResolver(testOwner, authn.WithInstancePlane(v, stubUsers{}))
if err != nil {
t.Fatalf("NewResolver: %v", err)
}
- return &fakeBackend{root: root, resolver: resolver, tokens: store}
+ return &fakeBackend{root: root, resolver: resolver}
}
-func (b *fakeBackend) ReposRoot() string { return b.root }
-func (b *fakeBackend) Resolver() *authn.Resolver { return b.resolver }
-func (b *fakeBackend) TokenStore() *service.TokenStore { return b.tokens }
+func (b *fakeBackend) ReposRoot() string { return b.root }
+func (b *fakeBackend) Resolver() *authn.Resolver { return b.resolver }
func (b *fakeBackend) ValidatePush(ctx context.Context, req service.PushRequest) error {
b.seen = append(b.seen, req)
@@ 204,7 240,7 @@ func bareRepo(t *testing.T, root string, ref core.SpaceRef) string {
return dir
}
-// errFakeStore is a store outage: not a bad credential, and must never be
+// errFakeStore is a backend outage: not a policy refusal, and must never be
// reported as one.
var errFakeStore = errors.New("fake store is down")
M hooks/hook_test.go => hooks/hook_test.go +8 -8
@@ 113,7 113,7 @@ func refLine(u RefUpdate) string { return u.Old + " " + u.New + " " + u.Ref + "\
// TestUpdateHookAcceptsAValidRef walks the two hooks of the rejecting path in
// the order git runs them.
func TestUpdateHookAcceptsAValidRef(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
rt, stderr := f.hookRuntime(t, []string{"hooks/pre-receive"}, refLine(mainUpdate), nil)
if code := Run(rt); code != 0 {
@@ 133,7 133,7 @@ func TestUpdateHookAcceptsAValidRef(t *testing.T) {
// TestUpdateHookPrintsTheRejection: this text is the whole user interface of a
// failed push, so the hook must print what the daemon wrote and exit non-zero.
func TestUpdateHookPrintsTheRejection(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
want := rejection("refs/heads/main", true, service.PushProblem{
Kind: service.ProblemFrontmatter,
Path: "specs/0002-broken.md",
@@ 161,7 161,7 @@ func TestUpdateHookPrintsTheRejection(t *testing.T) {
// TestHooksFailClosed is the rule the whole design rests on: with no daemon
// answering, a push is refused rather than accepted unvalidated.
func TestHooksFailClosed(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
dead := filepath.Join(f.root, "gone", "hook.sock")
for _, tt := range []struct {
@@ 193,7 193,7 @@ func TestHooksFailClosed(t *testing.T) {
// TestPostReceiveCannotReject: git ignores its exit status, so pretending
// otherwise would only produce noise. It warns and names the backstop.
func TestPostReceiveCannotReject(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
dead := filepath.Join(f.root, "gone", "hook.sock")
rt, stderr := f.hookRuntime(t, []string{"hooks/post-receive"}, refLine(mainUpdate),
@@ 208,7 208,7 @@ func TestPostReceiveCannotReject(t *testing.T) {
// TestHookRefusesAMisconfiguredEnvironment: no principal means nobody
// authorized the push, and there is nothing to default to.
func TestHookRefusesAMisconfiguredEnvironment(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
rt, stderr := f.hookRuntime(t,
[]string{"hooks/update", mainUpdate.Ref, mainUpdate.Old, mainUpdate.New}, "",
@@ 224,7 224,7 @@ func TestHookRefusesAMisconfiguredEnvironment(t *testing.T) {
}
func TestUpdateHookNeedsThreeArguments(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
rt, stderr := f.hookRuntime(t, []string{"hooks/update", "refs/heads/main"}, "", nil)
if code := Run(rt); code == 0 {
t.Fatal("update ran with the wrong number of arguments")
@@ 235,7 235,7 @@ func TestUpdateHookNeedsThreeArguments(t *testing.T) {
// TestPreReceiveForwardsPushOptions: the update hook never sees them, so
// whether skip-validation works at all depends on this handoff.
func TestPreReceiveForwardsPushOptions(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
rt, stderr := f.hookRuntime(t, []string{"hooks/pre-receive"}, refLine(mainUpdate),
map[string]string{
@@ 260,7 260,7 @@ func TestPreReceiveForwardsPushOptions(t *testing.T) {
}
func TestPreReceiveRefusesAMalformedRefList(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
rt, stderr := f.hookRuntime(t, []string{"hooks/pre-receive"}, "not a ref line\n", nil)
if code := Run(rt); code == 0 {
t.Fatal("pre-receive accepted a ref list it could not parse")
M hooks/server.go => hooks/server.go +41 -29
@@ 6,6 6,7 @@ import (
"fmt"
"log/slog"
"net"
+ "net/http"
"os"
"path/filepath"
"strings"
@@ 32,13 33,13 @@ type Backend interface {
// repository under.
ReposRoot() string
- // Resolver carries the instance owner username, the one identity an
- // "owner" credential can resolve to.
+ // Resolver carries the instance owner username — the one identity an
+ // "owner" credential can resolve to — and the tokens.sr.ht plane an agent's
+ // credential is validated on. It is the same resolver the HTTP surfaces
+ // authenticate through, which is the point: one plane, one implementation of
+ // "is this credential good?".
Resolver() *authn.Resolver
- // TokenStore is what an agent's token is validated against.
- TokenStore() *service.TokenStore
-
// ValidatePush answers whether one ref may move. A *service.PushRejection
// is a policy refusal whose Error() is the text to print; anything else is
// an infrastructure failure and must fail the push closed.
@@ 460,37 461,47 @@ func (s *Server) spaceFor(repo string) (core.SpaceRef, error) {
// The owner is not looked up: sshd authenticated the SSH key and the forced
// command asserted it, and there is exactly one owner on this instance, so the
// name comes from the resolver rather than from the wire — a hook cannot name
-// somebody else. An agent's token is checked on every push.
+// somebody else. An agent's credential is checked on every push.
+//
+// An agent goes through authn.Resolver.ResolveAgent, the same call the HTTP
+// surfaces reach through their middleware. This path used to read the
+// agent_token table directly, which is exactly how it ended up accepting a
+// credential the HTTP planes had already stopped being the only door for: two
+// implementations of one question, drifting. There is one credential plane now
+// and one implementation of checking it.
+//
+// The grant check is here rather than in ValidatePush because it is about the
+// credential and not about the ref: a push by an agent is a proposal by another
+// transport, so it needs spec:propose exactly as the REST and MCP write planes
+// do. The refs rule still runs afterwards, inside ValidatePush, and still
+// confines the agent to proposals/* — a grant does not replace it and cannot
+// widen it.
func (s *Server) principal(ctx context.Context, space core.SpaceRef, cred Credential) (authn.Principal, Response, bool) {
owner := s.backend.Resolver().Owner()
switch cred.Kind {
case PrincipalOwner:
return authn.Principal{Kind: authn.KindOwner, Owner: owner, CookieUser: owner}, Response{}, true
case PrincipalAgent:
- tok, err := authn.ResolveAgentToken(ctx, s.backend.TokenStore(), cred.Token)
+ p, err := s.backend.Resolver().ResolveAgent(ctx, cred.Token, cred.Agent, cred.Session)
if err != nil {
- if authn.IsAuthFailure(err) {
+ // StatusFor is the one table: a bad or foreign credential (401) and
+ // a good one this instance has nothing to grant (403) are policy
+ // refusals the pusher can read and act on; anything else means we
+ // could not check, and an unanswerable check fails the push closed
+ // rather than reading as a bad token.
+ if status := authn.StatusFor(err); status < http.StatusInternalServerError {
s.log.Warn("refused an agent push", "space", space.String(), "error", err)
return authn.Principal{}, rejectedResponse(badTokenMessage(space, err)), false
}
- // The store could not answer. That is not a bad credential and
- // must not read as one; fail the push closed instead.
- s.log.Error("could not validate an agent token", "space", space.String(), "error", err)
+ s.log.Error("could not validate an agent credential", "space", space.String(), "error", err)
return authn.Principal{}, errorResponse(
- "spec.sr.ht could not check the agent token presented with this push: %v", err), false
+ "spec.sr.ht could not check the credential presented with this push: %v", err), false
+ }
+ if err := p.Authorize(authn.ActionPropose); err != nil {
+ s.log.Warn("refused an agent push", "space", space.String(), "error", err)
+ return authn.Principal{}, rejectedResponse(badTokenMessage(space, err)), false
}
- // The local plane, and only it: a push arrives over SSH with a token in
- // a hook's environment, and this path checks it against agent_token and
- // nothing else. A tokens.sr.ht working token is not accepted here — the
- // two planes meet in authn.Resolver, which serves the HTTP surfaces.
- return authn.Principal{
- Kind: authn.KindAgent,
- Owner: owner,
- Agent: cred.Agent,
- Session: cred.Session,
- TokenName: tok.Name,
- Plane: authn.PlaneLocal,
- }, Response{}, true
+ return p, Response{}, true
default:
// Request.Validate rejected every other spelling already.
return authn.Principal{}, errorResponse("unknown principal kind %q", cred.Kind), false
@@ 593,15 604,16 @@ func unknownOptionMessage(space core.SpaceRef, unknown []string) string {
return b.String()
}
-// badTokenMessage is what an agent sees when its token does not authenticate.
-// It never echoes the token.
+// badTokenMessage is what an agent sees when its credential does not
+// authenticate or does not carry spec:propose. It never echoes the token.
func badTokenMessage(space core.SpaceRef, cause error) string {
var b strings.Builder
fmt.Fprintf(&b, "spec.sr.ht rejected this push.\n\n")
fmt.Fprintf(&b, " space: %s\n\n", space)
- fmt.Fprintf(&b, " the agent token presented with this push was refused:\n")
+ fmt.Fprintf(&b, " the agent credential presented with this push was refused:\n")
fmt.Fprintf(&b, " %v\n\n", cause)
- fmt.Fprintf(&b, "Nothing was written. Agents write through the REST and MCP planes,\n")
- fmt.Fprintf(&b, "not over git; a token that works there is not a git credential.\n")
+ fmt.Fprintf(&b, "Nothing was written. Agent credentials are tokens.sr.ht working tokens,\n")
+ fmt.Fprintf(&b, "the same ones the REST and MCP planes take, and a push needs the\n")
+ fmt.Fprintf(&b, "spec:propose grant just as those do.\n")
return b.String()
}
M hooks/server_test.go => hooks/server_test.go +115 -77
@@ 4,6 4,7 @@ import (
"context"
"errors"
"fmt"
+ "net/http"
"path/filepath"
"strings"
"testing"
@@ 11,7 12,6 @@ import (
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
- "sourcecraft.dev/bigbes/sr-ht-spec/db"
"sourcecraft.dev/bigbes/sr-ht-spec/service"
)
@@ 41,7 41,15 @@ type serverFixture struct {
srv *Server
}
-func newServerFixture(t *testing.T, tokens map[string]*db.AgentToken) *serverFixture {
+func newServerFixture(t *testing.T) *serverFixture {
+ t.Helper()
+ return newServerFixtureAgainst(t, fakeDaemonOrigin(t, http.StatusNoContent, false))
+}
+
+// newServerFixtureAgainst is the same fixture with the tokens.sr.ht origin
+// chosen by the caller, so a test can put a revoking or an unreachable daemon
+// behind the agent plane.
+func newServerFixtureAgainst(t *testing.T, origin string) *serverFixture {
t.Helper()
root := shortTempDir(t)
repo := filepath.Join(root, "~"+testSpace.Owner, testSpace.Name)
@@ 50,7 58,7 @@ func newServerFixture(t *testing.T, tokens map[string]*db.AgentToken) *serverFix
t.Fatalf("MkdirAll: %v", err)
}
}
- back := newFakeBackend(t, root, tokens)
+ back := newFakeBackendAgainst(t, root, origin)
srv, _ := startServer(t, back)
return &serverFixture{root: root, repo: repo, back: back, srv: srv}
}
@@ 71,7 79,7 @@ var mainUpdate = RefUpdate{Ref: "refs/heads/main", Old: oneOID, New: twoOID}
// TestPushLifecycle is the protocol as one push runs it: pre-receive records
// the options, update reads them back, post-receive lands.
func TestPushLifecycle(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
pre := f.request(MethodPushOptions, "4711", mainUpdate)
pre.Options = []string{OptionSkipValidation}
@@ 107,7 115,7 @@ func TestPushLifecycle(t *testing.T) {
// TestSkipValidationDefaultsOff proves the waiver is opt-in per push and does
// not leak from one push into the next.
func TestSkipValidationDefaultsOff(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
waived := f.request(MethodPushOptions, "1", mainUpdate)
waived.Options = []string{OptionSkipValidation}
@@ 133,7 141,7 @@ func TestSkipValidationDefaultsOff(t *testing.T) {
// half installed or the daemon restarted mid-push, and either deserves a
// sentence rather than a guess.
func TestUpdateWithoutPreReceiveIsRefused(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
resp := call(t, f.srv, f.request(MethodValidateRef, "4711", mainUpdate))
if resp.OK {
@@ 149,7 157,7 @@ func TestUpdateWithoutPreReceiveIsRefused(t *testing.T) {
// safe as a correlation key: a recycled pid would also have to be paired with
// an identical ref and object names.
func TestUpdateForAnUnannouncedRefIsRefused(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
call(t, f.srv, f.request(MethodPushOptions, "4711", mainUpdate))
other := RefUpdate{Ref: "refs/heads/proposals/9", Old: zeroOID, New: twoOID}
@@ 166,7 174,7 @@ func TestExpiredPushOptionsAreRefused(t *testing.T) {
if err := mkdirAll(repo); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
- back := newFakeBackend(t, root, nil)
+ back := newFakeBackend(t, root)
srv, _ := startServer(t, back, func(o *Options) { o.OptionTTL = time.Nanosecond })
f := &serverFixture{root: root, repo: repo, back: back, srv: srv}
@@ 181,7 189,7 @@ func TestExpiredPushOptionsAreRefused(t *testing.T) {
// ignoring a typo would reject the push for the very thing the human believed
// they had waived.
func TestUnknownPushOptionIsRejected(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
req := f.request(MethodPushOptions, "4711", mainUpdate)
req.Options = []string{"skip-validaton"}
@@ 200,7 208,7 @@ func TestUnknownPushOptionIsRejected(t *testing.T) {
// daemon owns, because the path is re-derived through gitx's layout rather
// than parsed out of what the hook claimed.
func TestRepositoryMustBeOurs(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
outside := []struct {
name string
@@ 231,7 239,7 @@ func TestRepositoryMustBeOurs(t *testing.T) {
// TestOwnerCredentialCannotNameSomebodyElse: the wire carries a kind, never a
// username. The owner comes from the resolver.
func TestOwnerCredentialCannotNameSomebodyElse(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
call(t, f.srv, f.request(MethodPushOptions, "1", mainUpdate))
call(t, f.srv, f.request(MethodValidateRef, "1", mainUpdate))
@@ 243,85 251,115 @@ func TestOwnerCredentialCannotNameSomebodyElse(t *testing.T) {
}
}
-func TestAgentCredential(t *testing.T) {
- live := &db.AgentToken{ID: 1, Name: "laptop", Hash: authn.HashToken("good"), Created: time.Now()}
- revokedAt := time.Now().Add(-time.Hour)
- revoked := &db.AgentToken{ID: 2, Name: "old", Hash: authn.HashToken("dead"), Revoked: &revokedAt}
- f := newServerFixture(t, map[string]*db.AgentToken{"good": live, "dead": revoked})
+// agentRequest is a push by an agent presenting token.
+func agentRequest(f *serverFixture, method Method, token string) Request {
+ req := f.request(method, "1", mainUpdate)
+ req.Credential = Credential{Kind: PrincipalAgent, Token: token, Agent: "claude/spec", Session: "s1"}
+ return req
+}
+
+// The push path takes a tokens.sr.ht working token, which is the whole point of
+// routing it through authn.Resolver: an agent can `git push` with the same
+// credential it uses on the REST and MCP planes.
+func TestAgentCredentialAcceptsAnInstanceToken(t *testing.T) {
+ f := newServerFixture(t)
+ tok := instanceToken("spec:propose")
- agent := func(token string) Request {
- req := f.request(MethodPushOptions, "1", mainUpdate)
- req.Credential = Credential{Kind: PrincipalAgent, Token: token, Agent: "claude/spec", Session: "s1"}
- return req
+ if resp := call(t, f.srv, agentRequest(f, MethodPushOptions, tok)); !resp.OK {
+ t.Fatalf("a valid instance token was refused: %+v", resp)
+ }
+ if resp := call(t, f.srv, agentRequest(f, MethodValidateRef, tok)); !resp.OK {
+ t.Fatalf("update: %+v", resp)
}
- t.Run("a valid token resolves to an agent", func(t *testing.T) {
- if resp := call(t, f.srv, agent("good")); !resp.OK {
- t.Fatalf("a valid agent token was refused: %+v", resp)
- }
- req := f.request(MethodValidateRef, "1", mainUpdate)
- req.Credential = Credential{Kind: PrincipalAgent, Token: "good", Agent: "claude/spec", Session: "s1"}
- if resp := call(t, f.srv, req); !resp.OK {
- t.Fatalf("update: %+v", resp)
- }
- p := f.back.seen[len(f.back.seen)-1].Principal
- if !p.IsAgent() || p.Agent != "claude/spec" || p.Session != "s1" || p.TokenName != "laptop" {
- t.Errorf("principal: %+v", p)
- }
- })
+ p := f.back.seen[len(f.back.seen)-1].Principal
+ if !p.IsAgent() || p.Agent != "claude/spec" || p.Session != "s1" {
+ t.Errorf("principal: %+v", p)
+ }
+ if p.Plane != authn.PlaneInstance {
+ t.Errorf("plane = %q, want the instance plane", p.Plane)
+ }
+ if p.TokenName != "tokens.sr.ht (stateless)" {
+ t.Errorf("TokenName = %q", p.TokenName)
+ }
+}
- t.Run("an unknown token is a rejection, and the token is not echoed", func(t *testing.T) {
- resp := call(t, f.srv, agent("wrong"))
- if resp.OK || !resp.Rejected {
- t.Fatalf("an unknown token was not rejected: %+v", resp)
- }
- mentionsNot(t, "the rejection", resp.Message, "wrong")
- })
+// The credential every agent used to push with — an opaque secret out of
+// agent_token — authenticates nowhere. It is not a token this instance sealed,
+// and there is no second store left to ask.
+func TestAgentCredentialRefusesTheOldOpaqueToken(t *testing.T) {
+ f := newServerFixture(t)
- t.Run("a revoked token says revoked", func(t *testing.T) {
- resp := call(t, f.srv, agent("dead"))
- if resp.OK || !resp.Rejected {
- t.Fatalf("a revoked token was not rejected: %+v", resp)
- }
- mentions(t, "the rejection", resp.Message, "revoked")
- })
+ resp := call(t, f.srv, agentRequest(f, MethodPushOptions, "s3cret-from-agent-token"))
+ if resp.OK || !resp.Rejected {
+ t.Fatalf("an opaque secret was not rejected: %+v", resp)
+ }
+ mentionsNot(t, "the rejection", resp.Message, "s3cret-from-agent-token")
}
-// TestTokenStoreOutageIsNotABadCredential: a store that cannot answer must
-// fail the push closed, not read as an invalid token — the difference between
-// "retry later" and "reprovision your agent".
-func TestTokenStoreOutageIsNotABadCredential(t *testing.T) {
- root := shortTempDir(t)
- repo := filepath.Join(root, "~"+testSpace.Owner, testSpace.Name)
- if err := mkdirAll(repo); err != nil {
- t.Fatalf("MkdirAll: %v", err)
+// A push is a proposal by another transport, so it needs spec:propose exactly
+// as the REST and MCP write planes do — and a token without it is refused
+// before any ref is looked at.
+func TestAgentCredentialRefusesATokenWithoutTheProposeGrant(t *testing.T) {
+ f := newServerFixture(t)
+
+ for _, grantString := range []string{"spec:read", "bench:upload"} {
+ t.Run(grantString, func(t *testing.T) {
+ resp := call(t, f.srv, agentRequest(f, MethodPushOptions, instanceToken(grantString)))
+ if resp.OK || !resp.Rejected {
+ t.Fatalf("a token without spec:propose was not rejected: %+v", resp)
+ }
+ mentions(t, "the rejection", resp.Message, "spec:propose")
+ })
}
- lookup := &fakeLookup{err: errFakeStore}
- store := service.NewTokenStore(lookup)
- resolver, err := authn.NewResolver(testOwner, store)
- if err != nil {
- t.Fatalf("NewResolver: %v", err)
+ if len(f.back.seen) != 0 {
+ t.Errorf("a refused credential reached the backend %d times", len(f.back.seen))
}
- back := &fakeBackend{root: root, resolver: resolver, tokens: store}
- srv, _ := startServer(t, back)
- f := &serverFixture{root: root, repo: repo, back: back, srv: srv}
+}
- req := f.request(MethodPushOptions, "1", mainUpdate)
- req.Credential = Credential{Kind: PrincipalAgent, Token: "anything"}
- resp := call(t, f.srv, req)
+// spec.sr.ht answers to one human on this instance, over git as over HTTP.
+func TestAgentCredentialRefusesATokenOfAnotherOwner(t *testing.T) {
+ f := newServerFixture(t)
+
+ resp := call(t, f.srv, agentRequest(f, MethodPushOptions, foreignToken("spec:propose")))
+ if resp.OK || !resp.Rejected {
+ t.Fatalf("a foreign owner's token was not rejected: %+v", resp)
+ }
+ mentions(t, "the rejection", resp.Message, "someone")
+}
+
+// A revoked token says revoked, so an operator can tell a credential they
+// killed from one that never existed.
+func TestAgentCredentialRefusesARevokedToken(t *testing.T) {
+ f := newServerFixtureAgainst(t, fakeDaemonOrigin(t, http.StatusNotFound, false))
+
+ resp := call(t, f.srv, agentRequest(f, MethodPushOptions, instanceToken("spec:propose id:42")))
+ if resp.OK || !resp.Rejected {
+ t.Fatalf("a revoked token was not rejected: %+v", resp)
+ }
+ mentions(t, "the rejection", resp.Message, "revoked")
+}
+
+// TestUnreachableDaemonIsNotABadCredential: a validator that cannot complete
+// its revocation check must fail the push closed, not read as an invalid token
+// — the difference between "retry later" and "reprovision your agent".
+func TestUnreachableDaemonIsNotABadCredential(t *testing.T) {
+ f := newServerFixtureAgainst(t, fakeDaemonOrigin(t, http.StatusNoContent, true))
+
+ resp := call(t, f.srv, agentRequest(f, MethodPushOptions, instanceToken("spec:propose id:42")))
if resp.OK {
- t.Fatal("a store outage let a push through")
+ t.Fatal("an unreachable tokens.sr.ht let a push through")
}
if resp.Rejected {
- t.Errorf("a store outage was reported as a bad credential: %+v", resp)
+ t.Errorf("an unreachable daemon was reported as a bad credential: %+v", resp)
}
- mentions(t, "the failure", resp.Error, "could not check the agent token")
+ mentions(t, "the failure", resp.Error, "could not check the credential")
}
// TestValidatePushRejectionIsPassedThroughVerbatim: the daemon composed the
// text for a terminal and this package must not reword it.
func TestValidatePushRejectionIsPassedThroughVerbatim(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
want := rejection("refs/heads/main", true, service.PushProblem{
Kind: service.ProblemFrontmatter,
Path: "specs/0002-broken.md",
@@ 342,7 380,7 @@ func TestValidatePushRejectionIsPassedThroughVerbatim(t *testing.T) {
// TestInfrastructureFailureIsNotAPolicyRejection keeps "you broke a rule"
// and "we broke" apart: only the first is worth changing your push over.
func TestInfrastructureFailureIsNotAPolicyRejection(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
f.back.validate = func(context.Context, service.PushRequest) error {
return fmt.Errorf("service: look up space: %w", errFakeStore)
}
@@ 366,7 404,7 @@ func TestPostReceiveReportsANotifierFailure(t *testing.T) {
if err := mkdirAll(repo); err != nil {
t.Fatalf("MkdirAll: %v", err)
}
- back := newFakeBackend(t, root, nil)
+ back := newFakeBackend(t, root)
srv, _ := startServer(t, back, func(o *Options) {
o.OnPush = func(context.Context, core.SpaceRef, []RefUpdate) error {
return errors.New("the indexer is not running")
@@ 383,7 421,7 @@ func TestPostReceiveReportsANotifierFailure(t *testing.T) {
func TestNewServerRequiresItsWiring(t *testing.T) {
root := shortTempDir(t)
- back := newFakeBackend(t, root, nil)
+ back := newFakeBackend(t, root)
ok := Options{Backend: back, Socket: SocketPath(root),
OnPush: func(context.Context, core.SpaceRef, []RefUpdate) error { return nil }}
@@ 422,7 460,7 @@ func TestNewServerRequiresItsWiring(t *testing.T) {
// push path from the first.
func TestListenRefusesToStealALiveSocket(t *testing.T) {
root := shortTempDir(t)
- back := newFakeBackend(t, root, nil)
+ back := newFakeBackend(t, root)
startServer(t, back)
second, err := NewServer(Options{
@@ 452,7 490,7 @@ func TestListenClearsADeadSocket(t *testing.T) {
t.Fatalf("write a stale socket: %v", err)
}
- back := newFakeBackend(t, root, nil)
+ back := newFakeBackend(t, root)
srv, err := NewServer(Options{
Backend: back, Socket: socket, Log: discardLogger(),
OnPush: func(context.Context, core.SpaceRef, []RefUpdate) error { return nil },
@@ 469,7 507,7 @@ func TestListenClearsADeadSocket(t *testing.T) {
// TestGarbageOnTheSocketIsAnswered: the peer may not be a hook at all, and a
// closed connection would leave a real hook guessing.
func TestGarbageOnTheSocketIsAnswered(t *testing.T) {
- f := newServerFixture(t, nil)
+ f := newServerFixture(t)
conn, err := dialUnix(f.srv.Socket())
if err != nil {
t.Fatalf("dial: %v", err)
M mcpsrv/grant_internal_test.go => mcpsrv/grant_internal_test.go +14 -16
@@ 33,12 33,11 @@ func instancePrincipal(t *testing.T, grantString string) authn.Principal {
return p
}
-// localPrincipal is agentPrincipal as spec's own agent_token resolves it.
-func localPrincipal() authn.Principal {
- p := agentPrincipal()
- p.Plane = authn.PlaneLocal
- return p
-}
+// cliPrincipal is agentPrincipal as `specsrht doc propose` builds it: an agent
+// a local process asserted, on no credential plane and therefore with no grant
+// set. The resolver never produces one — it is why Plane outlived the local
+// agent-token plane it used to distinguish.
+func cliPrincipal() authn.Principal { return agentPrincipal() }
// Every tool that serves content asks for spec:read, and asks for it per tool
// rather than at the Gate — /mcp carries the write tools too, and a surface-wide
@@ 72,12 71,12 @@ func TestReadToolsRequireTheReadGrant(t *testing.T) {
})
}
-// And every credential that carries no grants passes untouched, which is what
-// keeps the agents configured today working: the local agent token and the
-// owner's cookie have nothing to check.
+// And every principal that carries no grants passes untouched: the owner's
+// cookie is a person rather than a machine credential, and the CLI's agent
+// presented nothing to have a grant clipped out of.
func TestReadToolsPassEveryUngrantedCredential(t *testing.T) {
for name, p := range map[string]authn.Principal{
- "local agent token": localPrincipal(),
+ "locally asserted CLI agent": cliPrincipal(),
"owner cookie": {Kind: authn.KindOwner, Owner: "bigbes"},
"instance token, spec:read": instancePrincipal(t, "spec:read"),
"instance token, universal": instancePrincipal(t, "*"),
@@ 152,13 151,12 @@ func TestProposeToolRequiresTheProposeGrant(t *testing.T) {
}
}
-// And every credential that may propose clears it — the local agent token above
-// all, which carries no grants and must keep working exactly as it does today.
+// And every credential that may propose clears it.
func TestProposeToolAcceptsTheGrantedCredentials(t *testing.T) {
svc := realWriter(t)
for name, p := range map[string]authn.Principal{
- "local agent token": localPrincipal(),
+ "locally asserted CLI agent": cliPrincipal(),
"instance token, spec:propose": instancePrincipal(t, "spec:propose"),
"instance token, universal": instancePrincipal(t, "*"),
} {
@@ 177,12 175,12 @@ func TestProposeToolAcceptsTheGrantedCredentials(t *testing.T) {
// A grant is not a substitute for provenance. An instance token with the widest
// grant there is still cannot write without saying who wrote it — the refusal
// changes from authorization to provenance, and does not go away.
-func TestProposeToolStillDemandsProvenanceOnBothPlanes(t *testing.T) {
+func TestProposeToolStillDemandsProvenance(t *testing.T) {
svc := realWriter(t)
for name, p := range map[string]authn.Principal{
- "local": localPrincipal(),
- "instance": instancePrincipal(t, "*"),
+ "locally asserted CLI agent": cliPrincipal(),
+ "instance token": instancePrincipal(t, "*"),
} {
t.Run(name, func(t *testing.T) {
noSession := p
A migrations/0005_drop_agent_token.sql => migrations/0005_drop_agent_token.sql +30 -0
@@ 0,0 1,30 @@
+-- +brant Up
+
+-- Agent issuance moved to tokens.sr.ht, so spec.sr.ht stops holding a
+-- credential of its own.
+--
+-- agent_token was one instance-wide shared secret with no owner, no expiry and
+-- no grants: every agent on the instance presented the same string, and the only
+-- boundary around it was the refs rule (an agent credential may only move refs
+-- under proposals/*). That rule is untouched and still the boundary that
+-- matters; what changes is that the credential is now per-agent, owned, expiring
+-- and grant-carrying, and is verified by signature rather than by a lookup here.
+--
+-- DEPLOY GATE: applying this locks out every agent still configured with the
+-- shared secret, over HTTP and over `git push` alike. Every one of them must
+-- hold a tokens.sr.ht working token *before* this migration runs.
+--
+-- The rows are dropped with the table and the Down below cannot bring them back:
+-- only sha256 hashes were ever stored, and nothing derives a token from its
+-- hash. A rollback therefore restores the shape of the old plane and none of its
+-- credentials, which have to be re-issued.
+DROP TABLE agent_token;
+
+-- +brant Down
+CREATE TABLE agent_token (
+ id SERIAL PRIMARY KEY,
+ name TEXT NOT NULL,
+ token_hash BYTEA NOT NULL UNIQUE,
+ created TIMESTAMPTZ NOT NULL DEFAULT now(),
+ revoked TIMESTAMPTZ
+);
M schema.sql => schema.sql +5 -9
@@ 71,15 71,11 @@ CREATE TABLE proposal (
-- state-filtered, newest-first scans.
CREATE INDEX ix_proposal_state_created ON proposal (state, created DESC);
--- Only a hash is ever stored; the token itself exists exactly once, at mint
--- time, in the response to the operator.
-CREATE TABLE agent_token (
- id SERIAL PRIMARY KEY,
- name TEXT NOT NULL,
- token_hash BYTEA NOT NULL UNIQUE,
- created TIMESTAMPTZ NOT NULL DEFAULT now(),
- revoked TIMESTAMPTZ
-);
+-- There is no credential table. agent_token stood here — one instance-wide
+-- shared secret, stored as a sha256 — until agent issuance moved to
+-- tokens.sr.ht (migration 0005). A working token is signed by the instance and
+-- carries its own owner, expiry and grants, so authenticating one is a
+-- signature check in authn/ and there is nothing here to look up.
-- Index staleness: compared against the space's approved head.
CREATE TABLE index_stamp (
M service/bearer.go => service/bearer.go +18 -17
@@ 34,14 34,14 @@ type options struct {
haveConf bool
}
-// WithInstanceTokens offers the instance config to the tokens.sr.ht bearer
-// plane. Whether a plane is actually built depends on what is in it: see
-// instancePlane, which treats a missing [tokens.sr.ht] section as "there is no
-// such daemon on this instance" rather than as a misconfiguration.
+// WithInstanceTokens builds the tokens.sr.ht bearer plane from the instance
+// config. It is the only plane an agent can authenticate on, so passing this
+// option and having no [tokens.sr.ht] section is a startup failure — see
+// instancePlane.
//
-// It is an option rather than a parameter because the two CLI paths that build a
-// Service — `specsrht token` and `specsrht doc` — authenticate nobody and have
-// no use for a validator or the HTTP client behind it.
+// It is an option rather than a parameter because `specsrht doc` also builds a
+// Service, authenticates nobody, and has no use for a validator or the HTTP
+// client behind it.
func WithInstanceTokens(conf ini.File) Option {
return func(o *options) {
o.conf = conf
@@ 49,16 49,15 @@ func WithInstanceTokens(conf ini.File) Option {
}
}
-// instancePlane builds the tokens.sr.ht bearer plane from the instance config,
-// or reports that there is none to build (a nil option, nil error).
+// instancePlane builds the tokens.sr.ht bearer plane from the instance config.
//
-// The absence is the case worth spelling out. An instance whose config.ini has
-// no [tokens.sr.ht] section has no such daemon, and spec must start anyway and
-// keep accepting its own agent token: the local plane is not a fallback for a
-// broken instance plane, it is the plane this service shipped with. So a missing
-// origin is an answer, not an error — while an origin that is present and
-// unusable is an error, and fails startup where an operator is looking rather
-// than one request at a time as an unexplained 503.
+// A missing [tokens.sr.ht] origin used to be an answer rather than an error:
+// spec minted its own agent token, so an instance without the daemon simply kept
+// using the plane it shipped with. It no longer has one. A daemon that came up
+// without this plane would serve reads and refuse every agent write on the
+// instance — over HTTP and over `git push` alike — so the absence fails startup,
+// where an operator is looking, instead of surfacing one request at a time as an
+// unexplained 503.
//
// The origin is read in its internal form (GetOrigin's external=false), so the
// revocation check of SPEC ch. 6 step 4 crosses the docker network directly
@@ 66,7 65,9 @@ func WithInstanceTokens(conf ini.File) Option {
func instancePlane(conf ini.File, q db.Querier) (authn.ResolverOption, error) {
origin := config.GetOrigin(conf, TokensSection, false)
if origin == "" {
- return nil, nil
+ return nil, fmt.Errorf(
+ "service: no [%s] origin in config.ini; spec.sr.ht issues no agent credential of "+
+ "its own and cannot authenticate an agent without the daemon that does", TokensSection)
}
// The node id is what the daemon's internal guard logs the caller as. The
M service/bearer_test.go => service/bearer_test.go +52 -49
@@ 36,14 36,12 @@ func instanceAgent(t *testing.T, grantString string) authn.Principal {
return p
}
-// localAgent is agentPrincipal as spec's own agent_token resolves it: no owner
-// of its own, no grants, nothing to authorize against.
-func localAgent() authn.Principal {
- p := agentPrincipal()
- p.Plane = authn.PlaneLocal
- p.TokenName = "laptop"
- return p
-}
+// cliAgent is agentPrincipal as `specsrht doc propose` builds it: an agent a
+// local process asserted, on no credential plane and with no grant set, because
+// it presented no credential. It is the one agent principal the resolver never
+// produces, and the reason authn.Plane outlived the local plane it used to
+// distinguish.
+func cliAgent() authn.Principal { return agentPrincipal() }
// proposeRequest is a well-formed open, so that the only thing a test varies is
// the principal.
@@ 80,7 78,7 @@ func TestProposeRefusesAnInstanceTokenWithoutTheProposeGrant(t *testing.T) {
func TestProposeAcceptsTheGrantedPrincipals(t *testing.T) {
svc, _ := newService(t)
cases := map[string]authn.Principal{
- "local agent token": localAgent(),
+ "locally asserted CLI agent": cliAgent(),
"instance token, exact grant": instanceAgent(t, "spec:propose"),
"instance token, both grants": instanceAgent(t, "spec:read spec:propose"),
"instance token, universal": instanceAgent(t, "*"),
@@ 98,37 96,37 @@ func TestProposeAcceptsTheGrantedPrincipals(t *testing.T) {
}
}
-// The refs rule is untouched by grants and applies to both planes identically:
+// The refs rule is untouched by grants and by the removal of the local plane:
// an agent credential moves refs under the proposal prefix and nothing else, no
// matter how wide the grant set behind it is.
-func TestRefsRuleIsUnchangedOnBothPlanes(t *testing.T) {
- planes := map[string]authn.Principal{
- "local": localAgent(),
- "instance": instanceAgent(t, "*"),
+func TestRefsRuleIsUnchangedByGrants(t *testing.T) {
+ agents := map[string]authn.Principal{
+ "locally asserted CLI agent": cliAgent(),
+ "instance token, universal": instanceAgent(t, "*"),
}
newHash := plumbing.NewHash("1f0c1d1a1e2b3c4d5e6f708192a3b4c5d6e7f809")
- for name, p := range planes {
+ for name, p := range agents {
t.Run(name, func(t *testing.T) {
kind, err := principalKind(p)
require.NoError(t, err)
assert.Equal(t, gitx.PrincipalAgent, kind,
- "both planes are agents to the refs rule; a universal grant does not promote one")
+ "an agent is an agent to the refs rule; a universal grant does not promote one")
// Outside the proposal prefix: refused.
err = gitx.CheckRefUpdate(kind, gitx.DefaultApprovedBranch, gitx.RefUpdate{
Ref: "refs/heads/" + gitx.DefaultApprovedBranch, New: newHash, FastForward: true,
})
assert.ErrorIs(t, err, gitx.ErrRefRejected,
- "an agent may not move the approved branch on either plane")
+ "an agent may not move the approved branch")
err = gitx.CheckRefUpdate(kind, gitx.DefaultApprovedBranch, gitx.RefUpdate{
Ref: "refs/heads/scratch", New: newHash, FastForward: true,
})
assert.ErrorIs(t, err, gitx.ErrRefRejected,
- "an agent may not create a branch outside the proposal prefix on either plane")
+ "an agent may not create a branch outside the proposal prefix")
- // Inside it: permitted, on both planes.
+ // Inside it: permitted.
err = gitx.CheckRefUpdate(kind, gitx.DefaultApprovedBranch, gitx.RefUpdate{
Ref: "refs/heads/" + core.ProposalPrefix + "7", New: newHash, FastForward: true,
})
@@ 137,14 135,14 @@ func TestRefsRuleIsUnchangedOnBothPlanes(t *testing.T) {
}
}
-// Provenance is still mandatory on both planes. A grant says what a credential
-// was minted for; it says nothing about who wrote a commit, and it does not
-// excuse a write from saying so.
-func TestProvenanceStillRequiredOnBothPlanes(t *testing.T) {
+// Provenance is still mandatory. A grant says what a credential was minted for;
+// it says nothing about who wrote a commit, and it does not excuse a write from
+// saying so.
+func TestProvenanceStillRequired(t *testing.T) {
svc, _ := newService(t)
for name, p := range map[string]authn.Principal{
- "local": localAgent(),
- "instance": instanceAgent(t, "*"),
+ "locally asserted CLI agent": cliAgent(),
+ "instance token": instanceAgent(t, "*"),
} {
t.Run(name, func(t *testing.T) {
noSession := p
@@ 174,11 172,12 @@ func tokensConf(section ini.Section) ini.File {
return conf
}
-// An instance whose config has no [tokens.sr.ht] section has no such daemon.
-// The plane must then be absent rather than broken: spec starts, and its own
-// agent token is the only door — which is exactly what the instance ran before
-// tokens.sr.ht existed.
-func TestInstancePlane_AbsentSectionIsNotAnError(t *testing.T) {
+// A missing [tokens.sr.ht] origin used to be an answer — spec minted its own
+// credential, so an instance without the daemon simply used the plane it
+// shipped with. It has none now, so the absence is a startup failure: a daemon
+// that came up like this would serve reads and refuse every agent write on the
+// instance, over HTTP and over `git push` alike.
+func TestInstancePlane_AbsentSectionIsAStartupError(t *testing.T) {
for name, conf := range map[string]ini.File{
"no section at all": tokensConf(nil),
"section with no origin": tokensConf(ini.Section{
@@ 186,9 185,9 @@ func TestInstancePlane_AbsentSectionIsNotAnError(t *testing.T) {
}),
} {
t.Run(name, func(t *testing.T) {
- plane, err := instancePlane(conf, deadDB(t))
- require.NoError(t, err, "a missing origin is an answer, not a misconfiguration")
- assert.Nil(t, plane)
+ _, err := instancePlane(conf, deadDB(t))
+ require.Error(t, err, "there is no other plane left to fall back to")
+ assert.Contains(t, err.Error(), TokensSection)
})
}
}
@@ 203,7 202,7 @@ func TestInstancePlane_BuiltFromTheInternalOrigin(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, plane)
- rs, err := authn.NewResolver("bigbes", NewTokenStore(nil), plane)
+ rs, err := authn.NewResolver("bigbes", plane)
require.NoError(t, err)
assert.True(t, rs.HasInstancePlane())
}
@@ 216,20 215,24 @@ func TestInstancePlane_UnusableOriginIsAStartupError(t *testing.T) {
assert.Contains(t, err.Error(), "tokens.sr.ht")
}
-// Service.New wires the plane when it is offered and a config supports it, and
-// builds the same local-only resolver it always did when it is not.
-func TestNew_InstancePlaneIsOptional(t *testing.T) {
+// Service.New wires the plane when a config supports it, refuses when the
+// option is passed and the config does not, and builds a planeless resolver for
+// the CLI paths that do not ask for one.
+func TestNew_InstancePlaneWiring(t *testing.T) {
cfg := testConfig(t, t.TempDir())
+ // `specsrht doc`: authenticates nobody, so it asks for no plane and gets a
+ // resolver that refuses every credential rather than pretending to check
+ // one.
svc, err := New(cfg, deadDB(t))
require.NoError(t, err)
- assert.False(t, svc.Resolver().HasInstancePlane(),
- "no option means no instance plane, exactly as before it existed")
+ assert.False(t, svc.Resolver().HasInstancePlane())
- svc, err = New(cfg, deadDB(t), WithInstanceTokens(tokensConf(nil)))
- require.NoError(t, err)
- assert.False(t, svc.Resolver().HasInstancePlane(),
- "an instance without a [tokens.sr.ht] section must still start, on the local plane")
+ // The daemon: asks for the plane, and an instance that cannot give it one
+ // fails here rather than at every agent's first request.
+ _, err = New(cfg, deadDB(t), WithInstanceTokens(tokensConf(nil)))
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), TokensSection)
svc, err = New(cfg, deadDB(t), WithInstanceTokens(tokensConf(ini.Section{
"origin": "https://tokens.srht.bigb.es",
@@ 240,13 243,13 @@ func TestNew_InstancePlaneIsOptional(t *testing.T) {
// --- Postgres-backed integration tests (skip when SPECSRHT_TEST_PG is unset) ---
-// The whole open path, once per plane: the credential every agent is configured
-// with today and a tokens.sr.ht token carrying spec:propose both open a
-// proposal, with the same provenance recorded on it.
-func TestProposeOpensProposalOnBothPlanes(t *testing.T) {
+// The whole open path: a tokens.sr.ht token carrying spec:propose and the CLI's
+// locally asserted agent both open a proposal, with the same provenance
+// recorded on it.
+func TestProposeOpensProposalForEveryAgentPrincipal(t *testing.T) {
for name, p := range map[string]authn.Principal{
- "local agent token": localAgent(),
- "instance token": instanceAgent(t, "spec:read spec:propose"),
+ "locally asserted CLI agent": cliAgent(),
+ "instance token": instanceAgent(t, "spec:read spec:propose"),
} {
t.Run(name, func(t *testing.T) {
svc, _ := newTestService(t)
M service/service.go => service/service.go +10 -15
@@ 215,7 215,6 @@ type Service struct {
cfg Config
q db.Querier
store *db.Store
- tokens *TokenStore
resolver *authn.Resolver
// ownerUserID caches the id of the owner's "user" row, seeded by
@@ 242,13 241,14 @@ type Service struct {
// q is normally the *sql.DB the daemon opened from Config.ConnectionString and
// handed to core-go's database middleware, so request-scoped queries and the
// reconciler's background queries share one pool. A nil handle is refused
-// rather than tolerated: every agent token would then resolve as unknown, which
-// looks exactly like a mass revocation and is a miserable thing to debug.
+// rather than tolerated: half this layer would then fail one query at a time
+// instead of once, at startup, where an operator is looking.
//
-// Pass WithInstanceTokens(conf) to offer the resolver the tokens.sr.ht bearer
-// plane alongside the local agent-token one. Without it — and with it on an
-// instance whose config has no [tokens.sr.ht] section — the resolver knows only
-// the local plane, which is what every caller got before that plane existed.
+// Pass WithInstanceTokens(conf) to give the resolver the tokens.sr.ht plane —
+// the only plane an agent can authenticate on. The daemon passes it and fails
+// startup without a [tokens.sr.ht] origin; the CLI paths (`specsrht doc`)
+// authenticate nobody and omit it, and the resolver they get refuses every
+// bearer credential rather than pretending to check one.
func New(cfg Config, q db.Querier, opts ...Option) (*Service, error) {
if err := cfg.Validate(); err != nil {
return nil, err
@@ 267,22 267,17 @@ func New(cfg Config, q db.Querier, opts ...Option) (*Service, error) {
if err != nil {
return nil, err
}
- if plane != nil {
- ropts = append(ropts, plane)
- }
+ ropts = append(ropts, plane)
}
- store := db.NewStore(q)
- tokens := NewTokenStore(store)
- resolver, err := authn.NewResolver(cfg.Instance.OwnerName, tokens, ropts...)
+ resolver, err := authn.NewResolver(cfg.Instance.OwnerName, ropts...)
if err != nil {
return nil, fmt.Errorf("service: build resolver: %w", err)
}
return &Service{
cfg: cfg,
q: q,
- store: store,
- tokens: tokens,
+ store: db.NewStore(q),
resolver: resolver,
grace: DefaultReconcileGrace,
now: time.Now,
M service/service_test.go => service/service_test.go +2 -65
@@ 1,14 1,10 @@
package service
import (
- "context"
"errors"
"strings"
"testing"
"time"
-
- "sourcecraft.dev/bigbes/sr-ht-spec/authn"
- "sourcecraft.dev/bigbes/sr-ht-spec/db"
)
func TestLoadConfigAcceptsACompleteConfig(t *testing.T) {
@@ 104,65 100,6 @@ func TestNewValidatesTheConfig(t *testing.T) {
}
}
-// fakeTokens is an agentTokenLookup that answers from a script.
-type fakeTokens struct {
- row *db.AgentToken
- err error
-}
-
-func (f fakeTokens) AgentTokenByHash(context.Context, []byte) (*db.AgentToken, error) {
- return f.row, f.err
-}
-
-// The whole of the adapter is this error contract: db/ says ErrNotFound, authn
-// demands ErrUnknownToken, and an unmapped pass-through would turn a bad
-// credential into a 503 telling the agent to retry forever.
-func TestTokenStoreMapsAMissingRowToUnknownToken(t *testing.T) {
- ts := NewTokenStore(fakeTokens{err: db.ErrNotFound})
- _, err := ts.LookupAgentToken(context.Background(), []byte("hash"))
- if !errors.Is(err, authn.ErrUnknownToken) {
- t.Fatalf("err = %v, want authn.ErrUnknownToken", err)
- }
- if !authn.IsAuthFailure(err) {
- t.Error("an unknown token must be a permanent auth failure, not a transient one")
- }
-}
-
-func TestTokenStoreKeepsOtherFailuresTransient(t *testing.T) {
- boom := errors.New("connection refused")
- ts := NewTokenStore(fakeTokens{err: boom})
- _, err := ts.LookupAgentToken(context.Background(), []byte("hash"))
- if !errors.Is(err, boom) {
- t.Fatalf("err = %v, want it to wrap the store failure", err)
- }
- if authn.IsAuthFailure(err) {
- t.Error("a store outage must never read as a bad credential")
- }
-}
-
-// A revoked row is returned rather than refused, so authn can say "revoked"
-// instead of "unknown".
-func TestTokenStoreReturnsARevokedRow(t *testing.T) {
- revoked := fxTime(1)
- ts := NewTokenStore(fakeTokens{row: &db.AgentToken{
- ID: 7, Name: "cron", Hash: []byte("h"), Created: fxTime(0), Revoked: &revoked,
- }})
- tok, err := ts.LookupAgentToken(context.Background(), []byte("h"))
- if err != nil {
- t.Fatalf("LookupAgentToken: %v", err)
- }
- if !tok.IsRevoked() || tok.ID != 7 || tok.Name != "cron" {
- t.Fatalf("token = %+v", tok)
- }
-}
-
-func TestTokenStoreRefusesANilRowWithNoError(t *testing.T) {
- ts := NewTokenStore(fakeTokens{})
- if _, err := ts.LookupAgentToken(context.Background(), []byte("h")); err == nil {
- t.Fatal("a nil row with no error authenticated")
- }
-}
-
func TestServiceExposesItsWiring(t *testing.T) {
svc, root := newService(t)
if svc.ReposRoot() != root {
@@ 174,8 111,8 @@ func TestServiceExposesItsWiring(t *testing.T) {
if svc.Resolver() == nil || svc.Resolver().Owner() != "bigbes" {
t.Errorf("resolver = %v", svc.Resolver())
}
- if svc.Store() == nil || svc.TokenStore() == nil {
- t.Error("store or token store is nil")
+ if svc.Store() == nil {
+ t.Error("store is nil")
}
if svc.grace != DefaultReconcileGrace || svc.now == nil {
t.Errorf("reconciler defaults not wired: grace=%v", svc.grace)
D service/token.go => service/token.go +0 -197
@@ 1,197 0,0 @@
-package service
-
-import (
- "context"
- "errors"
- "fmt"
- "strings"
- "time"
- "unicode/utf8"
-
- "sourcecraft.dev/bigbes/sr-ht-spec/authn"
- "sourcecraft.dev/bigbes/sr-ht-spec/db"
-)
-
-// MaxTokenNameLen bounds a token's label. It is a human-readable note about
-// which agent holds the credential, rendered in a table and in `token list`;
-// the cap keeps a pasted paragraph from becoming a row nobody can read.
-const MaxTokenNameLen = 128
-
-// AgentTokenLookup is the one db/ method the token adapter needs. It is an
-// interface rather than a *db.Store so the error mapping below — the part that
-// actually carries a contract — can be tested against a fake instead of a
-// Postgres instance. *db.Store satisfies it.
-type AgentTokenLookup interface {
- AgentTokenByHash(ctx context.Context, hash []byte) (*db.AgentToken, error)
-}
-
-// TokenStore adapts db/'s agent_token queries to authn.TokenStore. It exists
-// because authn must not import db: authn declares the sliver of persistence it
-// needs, and service/ — the layer that is allowed to know about both — wires
-// them together.
-//
-// The whole of the adaptation is the error contract, and it is not cosmetic.
-// authn's contract is that "no such token" is an error satisfying
-// errors.Is(err, authn.ErrUnknownToken) and that everything else is transient.
-// db/ spells the same condition ErrNotFound, which authn has never heard of, so
-// an unmapped pass-through would make an unknown token look like a Postgres
-// outage: a 503 telling an agent to retry a credential that will never work.
-type TokenStore struct {
- lookup AgentTokenLookup
-}
-
-// NewTokenStore wires a db.Store in as authn's TokenStore.
-func NewTokenStore(lookup AgentTokenLookup) *TokenStore {
- return &TokenStore{lookup: lookup}
-}
-
-// TokenStore returns the adapter the resolver authenticates agents through.
-func (s *Service) TokenStore() *TokenStore { return s.tokens }
-
-// LookupAgentToken implements authn.TokenStore.
-//
-// A revoked row is returned rather than refused: authn is what turns it into a
-// refusal, so the refusal can say "revoked" instead of "unknown" and an
-// operator can tell a token they deliberately killed from one that never
-// existed. Any other failure is returned wrapped and unclassified, which authn
-// reads as transient — the fail-closed direction, since a store outage must
-// never read as a valid credential.
-func (t *TokenStore) LookupAgentToken(ctx context.Context, hash []byte) (authn.AgentToken, error) {
- if t.lookup == nil {
- return authn.AgentToken{}, errors.New("service: TokenStore has no backing store")
- }
- row, err := t.lookup.AgentTokenByHash(ctx, hash)
- if err != nil {
- if errors.Is(err, db.ErrNotFound) {
- return authn.AgentToken{}, fmt.Errorf("%w: no agent_token row matches the presented token",
- authn.ErrUnknownToken)
- }
- return authn.AgentToken{}, fmt.Errorf("service: look up agent token: %w", err)
- }
- if row == nil {
- // db/ never returns (nil, nil); a store that did would otherwise
- // authenticate a nil row as a valid token.
- return authn.AgentToken{}, errors.New("service: agent token lookup returned no row and no error")
- }
- return authn.AgentToken{
- ID: int64(row.ID),
- Name: row.Name,
- Hash: row.Hash,
- Created: row.Created,
- Revoked: row.Revoked,
- }, nil
-}
-
-// AgentToken is one credential as the surfaces above this layer need it: no
-// hash, because nothing above service/ has any use for it, and no plaintext,
-// because it exists only in the response to the call that minted it.
-type AgentToken struct {
- ID int
- Name string
- Created time.Time
- Revoked *time.Time
-}
-
-// Active reports whether the token may still authenticate.
-func (t AgentToken) Active() bool { return t.Revoked == nil }
-
-// IssueAgentToken mints a credential for the agent write plane and returns the
-// plaintext exactly once, alongside the stored row.
-//
-// Owner-only, and that is the interesting half of the ACL: an agent holding a
-// valid token may not mint another. Were it allowed to, revoking a compromised
-// credential would not end the compromise — the holder would simply have issued
-// itself a second one — and "revoke the token" is the entire incident response
-// this design has.
-//
-// The plaintext is returned rather than stored. There is no second chance to
-// read it, which is what makes a leaked database dump unreplayable, so a caller
-// that drops the value has to mint a new token.
-func (s *Service) IssueAgentToken(ctx context.Context, p authn.Principal, name string) (string, AgentToken, error) {
- if !p.IsOwner() {
- return "", AgentToken{}, fmt.Errorf("%w: %s may not issue agent tokens; only the instance owner may",
- ErrForbidden, p)
- }
- name, err := validateTokenName(name)
- if err != nil {
- return "", AgentToken{}, err
- }
-
- token, err := db.GenerateToken()
- if err != nil {
- return "", AgentToken{}, fmt.Errorf("service: %w", err)
- }
- row, err := s.store.CreateAgentToken(ctx, name, db.HashToken(token))
- if err != nil {
- return "", AgentToken{}, fmt.Errorf("service: issue agent token %q: %w", name, err)
- }
- return token, tokenView(row), nil
-}
-
-// ListAgentTokens returns every token, newest first, so the owner can see what
-// exists and pick one to revoke. Owner-only for the same reason minting is: the
-// list is the inventory of who can write, and an agent has no business reading
-// it.
-func (s *Service) ListAgentTokens(ctx context.Context, p authn.Principal) ([]AgentToken, error) {
- if !p.IsOwner() {
- return nil, fmt.Errorf("%w: %s may not list agent tokens; only the instance owner may", ErrForbidden, p)
- }
- rows, err := s.store.ListAgentTokens(ctx)
- if err != nil {
- return nil, fmt.Errorf("service: list agent tokens: %w", err)
- }
- out := make([]AgentToken, 0, len(rows))
- for _, row := range rows {
- out = append(out, tokenView(row))
- }
- return out, nil
-}
-
-// RevokeAgentToken stamps a token revoked. Owner-only. Revoking is a stamp
-// rather than a delete so the audit trail keeps naming the token that made past
-// proposals; re-revoking is a no-op, because an operator killing a credential
-// twice is not an error worth failing.
-func (s *Service) RevokeAgentToken(ctx context.Context, p authn.Principal, id int) error {
- if !p.IsOwner() {
- return fmt.Errorf("%w: %s may not revoke agent tokens; only the instance owner may", ErrForbidden, p)
- }
- if id <= 0 {
- return fmt.Errorf("%w: %d is not an agent token id", ErrInvalid, id)
- }
- if err := s.store.RevokeAgentToken(ctx, id); err != nil {
- if errors.Is(err, db.ErrNotFound) {
- return fmt.Errorf("%w: agent token %d", ErrNotFound, id)
- }
- return fmt.Errorf("service: revoke agent token %d: %w", id, err)
- }
- return nil
-}
-
-// validateTokenName normalizes and checks a token label. The rules are the
-// weakest ones that keep the listing readable and unambiguous: trimmed,
-// non-empty, valid UTF-8, no control characters, and bounded. A name grants
-// nothing, so nothing stricter would be buying anything.
-func validateTokenName(name string) (string, error) {
- name = strings.TrimSpace(name)
- if name == "" {
- return "", fmt.Errorf("%w: a token needs a name saying which agent holds it", ErrInvalid)
- }
- if len(name) > MaxTokenNameLen {
- return "", fmt.Errorf("%w: token name is %d bytes, over the %d-byte limit",
- ErrInvalid, len(name), MaxTokenNameLen)
- }
- if !utf8.ValidString(name) {
- return "", fmt.Errorf("%w: token name is not valid UTF-8", ErrInvalid)
- }
- for _, r := range name {
- if r < 0x20 || r == 0x7f {
- return "", fmt.Errorf("%w: token name contains a control character %U", ErrInvalid, r)
- }
- }
- return name, nil
-}
-
-// tokenView maps a stored token onto the surface shape, dropping the hash.
-func tokenView(row *db.AgentToken) AgentToken {
- return AgentToken{ID: row.ID, Name: row.Name, Created: row.Created, Revoked: row.Revoked}
-}
D service/token_acl_test.go => service/token_acl_test.go +0 -107
@@ 1,107 0,0 @@
-package service
-
-import (
- "errors"
- "strings"
- "testing"
-
- "sourcecraft.dev/bigbes/sr-ht-spec/authn"
-)
-
-// The three token calls are owner-only, and these tests run against a database
-// that cannot be reached: a refusal that needs a query is a refusal that would
-// have leaked the inventory, or minted the row, before deciding.
-
-func TestIssueAgentTokenIsOwnerOnly(t *testing.T) {
- svc, _ := newService(t)
-
- for name, p := range map[string]authn.Principal{
- "an agent": {Kind: authn.KindAgent, Owner: "bigbes", Agent: "claude", Session: "s1"},
- "anonymous": authn.Anonymous(),
- "a zero principal": {},
- } {
- token, row, err := svc.IssueAgentToken(t.Context(), p, "another")
- if !errors.Is(err, ErrForbidden) {
- t.Errorf("%s was not refused with ErrForbidden: %v", name, err)
- }
- if token != "" || row.ID != 0 {
- t.Errorf("%s got a token back: %q %+v", name, token, row)
- }
- }
-}
-
-// TestIssueAgentTokenRefusesAnAgentBeforeMinting is the property revocation
-// depends on: an agent that could mint would survive having its own credential
-// revoked, so "revoke the token" would stop being incident response.
-func TestIssueAgentTokenRefusesAnAgentBeforeMinting(t *testing.T) {
- svc, _ := newService(t)
- agent := authn.Principal{Kind: authn.KindAgent, Owner: "bigbes", Agent: "claude", Session: "s1"}
-
- _, _, err := svc.IssueAgentToken(t.Context(), agent, "self-issued")
- if !errors.Is(err, ErrForbidden) {
- t.Fatalf("an agent minting a token was not refused: %v", err)
- }
- if !strings.Contains(err.Error(), "only the instance owner") {
- t.Errorf("the refusal does not say who may: %v", err)
- }
-}
-
-func TestListAndRevokeAreOwnerOnly(t *testing.T) {
- svc, _ := newService(t)
- agent := authn.Principal{Kind: authn.KindAgent, Owner: "bigbes", Agent: "claude", Session: "s1"}
-
- if _, err := svc.ListAgentTokens(t.Context(), agent); !errors.Is(err, ErrForbidden) {
- t.Errorf("an agent listing tokens was not refused: %v", err)
- }
- if err := svc.RevokeAgentToken(t.Context(), agent, 1); !errors.Is(err, ErrForbidden) {
- t.Errorf("an agent revoking a token was not refused: %v", err)
- }
-}
-
-// TestRevokeAgentTokenChecksTheIDBeforeTheDatabase keeps a mistyped id from
-// becoming an UPDATE that matches nothing and reports "not found", which reads
-// like the token is already gone.
-func TestRevokeAgentTokenChecksTheIDBeforeTheDatabase(t *testing.T) {
- svc, _ := newService(t)
- owner := authn.Principal{Kind: authn.KindOwner, Owner: "bigbes"}
-
- for _, id := range []int{0, -1} {
- if err := svc.RevokeAgentToken(t.Context(), owner, id); !errors.Is(err, ErrInvalid) {
- t.Errorf("revoking id %d was not refused as invalid: %v", id, err)
- }
- }
-}
-
-func TestValidateTokenName(t *testing.T) {
- got, err := validateTokenName(" claude-code ")
- if err != nil {
- t.Fatalf("validateTokenName: %v", err)
- }
- if got != "claude-code" {
- t.Errorf("name = %q want it trimmed", got)
- }
-
- for name, in := range map[string]string{
- "empty": "",
- "only whitespace": " ",
- "a control character": "claude\x00code",
- "a newline": "claude\ncode",
- "longer than the caps": strings.Repeat("x", MaxTokenNameLen+1),
- } {
- if _, err := validateTokenName(in); !errors.Is(err, ErrInvalid) {
- t.Errorf("%s was accepted as a token name", name)
- }
- }
-}
-
-// TestIssueAgentTokenValidatesTheNameBeforeMinting keeps a rejected name from
-// consuming entropy and, more importantly, from leaving a row whose label the
-// owner cannot read in the listing.
-func TestIssueAgentTokenValidatesTheNameBeforeMinting(t *testing.T) {
- svc, _ := newService(t)
- owner := authn.Principal{Kind: authn.KindOwner, Owner: "bigbes"}
-
- if _, _, err := svc.IssueAgentToken(t.Context(), owner, " "); !errors.Is(err, ErrInvalid) {
- t.Errorf("a blank token name was accepted: %v", err)
- }
-}
M web/grant_test.go => web/grant_test.go +9 -10
@@ 28,9 28,6 @@ func mustGrants(t *testing.T, s string) grants.Grants {
// resolver's business, and this file is about what the handlers do with it.
func grantRouter(t *testing.T, p authn.Principal) http.Handler {
t.Helper()
- resolver, err := authn.NewResolver("bigbes", stubTokenStore{})
- require.NoError(t, err)
-
srv, err := New(Options{
Conf: ini.File{
"sr.ht": ini.Section{
@@ 39,13 36,14 @@ func grantRouter(t *testing.T, p authn.Principal) http.Handler {
"environment": "development",
"owner-name": "bigbes",
},
- "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
- "spec.sr.ht": ini.Section{"origin": "https://spec.example"},
- "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
+ "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
+ "spec.sr.ht": ini.Section{"origin": "https://spec.example"},
+ "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
+ "tokens.sr.ht": ini.Section{"origin": "https://tokens.example"},
},
Reader: newFakeReader(),
Searcher: &fakeSearcher{},
- Resolver: resolver,
+ Resolver: testResolver(t),
})
require.NoError(t, err)
@@ 95,11 93,12 @@ func TestReadRoutesRequireTheReadGrant(t *testing.T) {
}
})
- // The credentials that work today, and the instance token that was minted
- // for reading: all served, no 403 anywhere.
+ // Everything that may read: the owner's cookie, an agent a local process
+ // asserted (no credential, so no grant to read), and the instance tokens
+ // minted for reading. All served, no 403 anywhere.
for name, p := range map[string]authn.Principal{
"owner cookie": {Kind: authn.KindOwner, Owner: "bigbes", CookieUser: "bigbes"},
- "local agent token": {Kind: authn.KindAgent, Owner: "bigbes", Plane: authn.PlaneLocal, TokenName: "laptop"},
+ "locally asserted agent": {Kind: authn.KindAgent, Owner: "bigbes"},
"instance token, spec:read": instancePrincipal(t, "spec:read"),
"instance token, universal": instancePrincipal(t, "*"),
} {
M web/reader.go => web/reader.go +3 -22
@@ 83,16 83,9 @@ type Reader interface {
ReplyTo(ctx context.Context, p authn.Principal, threadID int, body string) (service.Comment, error)
ResolveThread(ctx context.Context, p authn.Principal, threadID int, resolved bool) error
- // ListTokens, IssueToken and RevokeToken are the agent-credential surface
- // behind /tokens. Like the comment writes they carry the principal
- // explicitly, because the authority is not the read ACL: all three are
- // owner-only, and an agent — authenticated though it is — may not read the
- // inventory of who can write, let alone mint itself a second credential.
- // IssueToken returns the plaintext exactly once; it is not stored and no
- // later call can produce it again.
- ListTokens(ctx context.Context, p authn.Principal) ([]service.AgentToken, error)
- IssueToken(ctx context.Context, p authn.Principal, name string) (string, service.AgentToken, error)
- RevokeToken(ctx context.Context, p authn.Principal, id int) error
+ // There is no token surface here. /tokens used to mint, list and revoke
+ // spec's own agent credential; issuance is tokens.sr.ht's now, so the route
+ // is a redirect and this interface has nothing to serve it.
// Inbox is every open proposal on the instance, newest first — the review
// queue. Digest is the recently policy-merged proposals, the firehose a human
@@ 192,18 185,6 @@ func (r serviceReader) ResolveThread(ctx context.Context, p authn.Principal, thr
return r.svc.ResolveThread(ctx, p, threadID, resolved)
}
-func (r serviceReader) ListTokens(ctx context.Context, p authn.Principal) ([]service.AgentToken, error) {
- return r.svc.ListAgentTokens(ctx, p)
-}
-
-func (r serviceReader) IssueToken(ctx context.Context, p authn.Principal, name string) (string, service.AgentToken, error) {
- return r.svc.IssueAgentToken(ctx, p, name)
-}
-
-func (r serviceReader) RevokeToken(ctx context.Context, p authn.Principal, id int) error {
- return r.svc.RevokeAgentToken(ctx, p, id)
-}
-
func (r serviceReader) Inbox(ctx context.Context) ([]service.Proposal, error) {
return r.svc.InboxProposals(ctx)
}
M web/router.go => web/router.go +4 -4
@@ 43,11 43,11 @@ func (s *Server) Register(r chi.Router) {
r.Get("/inbox", s.handleInbox)
r.Post("/inbox/seen", s.handleInboxSeen)
- // The token routes are owner-only and live at the root, not under a space:
- // there is one agent credential for the instance, not one per space.
+ // /tokens redirects to tokens.sr.ht, which issues every agent credential on
+ // the instance. The POST routes that minted and revoked here went with the
+ // table behind them; the GET stays so that a bookmark, the dashboard button
+ // and every doc that ever said "see /tokens" still land somewhere useful.
r.Get("/tokens", s.handleTokens)
- r.Post("/tokens", s.handleTokenIssue)
- r.Post("/tokens/{id}/revoke", s.handleTokenRevoke)
// The proposal routes are registered before the document wildcard. chi gives
// the static "p" segment priority over the "*" catch-all regardless, but
M web/server.go => web/server.go +18 -4
@@ 24,10 24,10 @@
// serving drafts by default would poison every downstream agent context with
// unreviewed text.
//
-// Two pages sit outside the space grammar: /inbox is the review queue, and
-// /tokens is the owner-only agent-credential page — mint (shown once), list,
-// revoke. There is one credential for the instance rather than one per space,
-// so it has no owner or space in its address.
+// Two routes sit outside the space grammar: /inbox is the review queue, and
+// /tokens redirects to tokens.sr.ht, which issues every agent credential on the
+// instance. /tokens was spec's own mint-list-revoke page until that credential
+// stopped being spec's to mint.
//
// # Who may read
//
@@ 74,6 74,12 @@ import (
// with an immutable cache lifetime (the hash changes whenever the bytes do).
var hashedCSSRe = regexp.MustCompile(`^main\.min\.[0-9a-f]{6,}\.css$`)
+// tokensSection is tokens.sr.ht's config section, spelled the way the instance's
+// config.ini spells it. service.TokensSection is the same string read for the
+// internal origin; this package cannot import service/ (the dependency arrow
+// runs the other way), so the constant is here rather than shared.
+const tokensSection = "tokens.sr.ht"
+
// Options is everything a Server needs. Every field is required; New says which
// one is missing rather than failing later inside a handler.
type Options struct {
@@ 109,6 115,13 @@ type Server struct {
hubOrigin string
cssHref string
+ // tokensOrigin is [tokens.sr.ht] origin in its *external* form. The only
+ // thing this package does with it is redirect a browser there, and a browser
+ // cannot reach the internal origin the bearer validator uses. Empty when the
+ // instance config has no such section, which handleTokens answers rather
+ // than papers over with a redirect to nowhere.
+ tokensOrigin string
+
nav []navItem
staticFileServer http.Handler
}
@@ 166,6 179,7 @@ func New(opts Options) (*Server, error) {
metaOrigin: metaOrigin,
origin: origin,
hubOrigin: config.GetOrigin(opts.Conf, "hub.sr.ht", true),
+ tokensOrigin: config.GetOrigin(opts.Conf, tokensSection, true),
cssHref: cssHref,
nav: buildNav(opts.Conf),
staticFileServer: http.StripPrefix("/static/", http.FileServer(http.FS(staticSub))),
M web/templates.go => web/templates.go +1 -1
@@ 46,7 46,7 @@ var funcMap = template.FuncMap{
}
// pageNames are the content templates; each is parsed with layout.html.
-var pageNames = []string{"index", "space", "document", "search", "error", "proposal", "inbox", "tokens"}
+var pageNames = []string{"index", "space", "document", "search", "error", "proposal", "inbox"}
// pages maps a page name to its parsed template set (layout + partials + that
// page). threads.html is parsed into every set rather than only into the
M web/templates/index.html => web/templates/index.html +1 -1
@@ 8,7 8,7 @@
<code>?rev=<sha></code> to pin any page to an immutable one.
</p>
<a href="/inbox" class="btn btn-primary btn-block">Review queue</a>
- <a href="/tokens" class="btn btn-default btn-block">Agent tokens</a>
+ <a href="/tokens" class="btn btn-default btn-block">Agent tokens →</a>
<form method="GET" action="/search" class="mt-3">
<div class="form-group">
<label class="sr-only" for="q">Query</label>
D web/templates/tokens.html => web/templates/tokens.html +0 -68
@@ 1,68 0,0 @@
-{{define "content"}}
-<div class="row">
- <div class="col-md-12">
- <h2>Agent tokens</h2>
- <p>
- The credential an agent presents as
- <code>Authorization: Bearer <token></code> to propose over the REST
- and MCP write planes. Only the stored hash lives in the database, so a
- token is shown exactly once — when it is minted.
- </p>
-
- {{if .Data.Minted}}
- <div class="alert alert-success">
- <p>
- <strong>{{.Data.MintedName}}</strong> minted. Copy it now — this is
- the only time it is shown.
- </p>
- <pre class="mb-0"><code>{{.Data.Minted}}</code></pre>
- </div>
- {{end}}
-
- <form method="POST" action="/tokens" class="form-inline mb-4">
- <div class="form-group">
- <label class="sr-only" for="name">Name</label>
- <input class="form-control" type="text" id="name" name="name"
- placeholder="which agent holds it" autocomplete="off" required>
- </div>
- <button class="btn btn-primary" type="submit">Mint token</button>
- </form>
-
- {{if .Data.Tokens}}
- <table class="table">
- <thead>
- <tr><th>Name</th><th>Created</th><th>State</th><th></th></tr>
- </thead>
- <tbody>
- {{range .Data.Tokens}}
- <tr>
- <td>{{.Name}}</td>
- <td class="text-muted"><code>{{.Created}}</code></td>
- <td>
- {{if .Active}}
- <span class="badge badge-success">active</span>
- {{else}}
- <span class="badge badge-danger">revoked</span>
- <small class="text-muted"><code>{{.Revoked}}</code></small>
- {{end}}
- </td>
- <td>
- {{if .Active}}
- <form method="POST" action="/tokens/{{.ID}}/revoke">
- <button type="submit" class="btn btn-sm btn-outline-danger">Revoke</button>
- </form>
- {{end}}
- </td>
- </tr>
- {{end}}
- </tbody>
- </table>
- {{else}}
- <p class="text-muted">
- No tokens yet. Without one the agent write plane refuses every caller as
- anonymous, so nothing can propose.
- </p>
- {{end}}
- </div>
-</div>
-{{end}}
M web/tokens.go => web/tokens.go +34 -137
@@ 2,146 2,43 @@ package web
import (
"net/http"
- "strconv"
- "time"
-
- "github.com/go-chi/chi/v5"
-
- "sourcecraft.dev/bigbes/sr-ht-spec/authn"
- "sourcecraft.dev/bigbes/sr-ht-spec/service"
)
-// tokensData is the agent-token page: the inventory, and — on the one response
-// that follows a mint — the plaintext that will never be shown again.
-//
-// Minted is empty on every other render. It is a field on the page rather than
-// a flash cookie or a redirect parameter on purpose: a secret in a URL lands in
-// the browser history and in any proxy log between here and the operator, and a
-// secret in a cookie is a secret stored twice.
-type tokensData struct {
- Tokens []tokenRow
- Minted string
- MintedName string
-}
-
-// tokenRow is one credential as a listing line. There is no hash column:
-// service.AgentToken carries no hash, which is the layer saying that nothing
-// above it has any business with the stored value.
-type tokenRow struct {
- ID int
- Name string
- Created string
- Revoked string // empty while the token is still active
- Active bool
-}
+// tokensPath is the page at tokens.sr.ht this service points a human at. SPEC
+// ch. 7 pins it: the daemon serves one page, `/tokens`, behind the unified-login
+// cookie, and "services со временем просто ссылаются сюда" — which is what this
+// handler is.
+const tokensPath = "/tokens"
-// handleTokens renders the token inventory.
+// handleTokens sends a human to tokens.sr.ht.
//
-// Owner-only, and the refusal is a 403 rather than the read plane's login
-// redirect: an agent reaching this page is authenticated already, so redirecting
-// it to log in would answer a question it did not ask. An anonymous browser is
-// sent to meta the usual way, because for a human the answer really is "log in".
-func (s *Server) handleTokens(w http.ResponseWriter, r *http.Request) {
- p := authn.PrincipalFromContext(r.Context())
- if p.IsAnonymous() {
- s.loginRedirect(w, r)
- return
- }
- if !p.IsOwner() {
- s.renderError(w, r, http.StatusForbidden, "only the instance owner may manage agent tokens")
- return
- }
- s.renderTokens(w, r, p, tokensData{})
-}
-
-// handleTokenIssue mints a token and renders the page with the plaintext shown
-// once.
+// This route used to be spec's own agent-credential page: mint (shown once),
+// list, revoke, all against the agent_token table. That table is gone and
+// issuance is centralised, so what is left of the route is the one thing it can
+// still honestly do — point at the place that issues the credential — and it is
+// a redirect rather than a page of prose because an operator who typed /tokens
+// wants the form, not an explanation of where the form moved to.
//
-// This is the one write in this package that does not end in a
-// post-redirect-get. A redirect would either drop the secret — the whole point
-// of the request — or carry it in a URL. So the POST renders, and the form's
-// name field is what a reload would re-submit: minting a second token by
-// accident is recoverable in one click on this very page, whereas a lost token
-// is not recoverable at all.
-func (s *Server) handleTokenIssue(w http.ResponseWriter, r *http.Request) {
- p, ok := s.tokenWriter(w, r)
- if !ok {
- return
- }
- token, row, err := s.reader.IssueToken(r.Context(), p, r.FormValue("name"))
- if err != nil {
- s.fail(w, r, err)
- return
- }
- s.renderTokens(w, r, p, tokensData{Minted: token, MintedName: row.Name})
-}
-
-// handleTokenRevoke stamps a token revoked and redirects back to the listing.
-func (s *Server) handleTokenRevoke(w http.ResponseWriter, r *http.Request) {
- p, ok := s.tokenWriter(w, r)
- if !ok {
- return
- }
- id, err := strconv.Atoi(chi.URLParam(r, "id"))
- if err != nil || id <= 0 {
- s.renderError(w, r, http.StatusNotFound, "no such agent token")
- return
- }
- if err := s.reader.RevokeToken(r.Context(), p, id); err != nil {
- s.fail(w, r, err)
- return
- }
- http.Redirect(w, r, "/tokens", http.StatusSeeOther)
-}
-
-// tokenWriter is the shared gate on both token writes: owner-only, and the same
-// cross-site guard approve/reject use — the CSRF defense a form post needs when
-// the session cookie is meta's and this service cannot set its SameSite. It
-// answers the request itself when it refuses, so a caller only checks ok.
-func (s *Server) tokenWriter(w http.ResponseWriter, r *http.Request) (authn.Principal, bool) {
- p := authn.PrincipalFromContext(r.Context())
- if !p.IsOwner() {
- s.renderError(w, r, http.StatusForbidden, "only the instance owner may manage agent tokens")
- return authn.Principal{}, false
- }
- if !s.sameOrigin(r) {
- s.renderError(w, r, http.StatusForbidden, "this request did not originate from this site")
- return authn.Principal{}, false
- }
- return p, true
-}
-
-// renderTokens reads the inventory and renders the page, carrying through
-// whatever the caller already has to show (a freshly minted token, or nothing).
-func (s *Server) renderTokens(w http.ResponseWriter, r *http.Request, p authn.Principal, data tokensData) {
- tokens, err := s.reader.ListTokens(r.Context(), p)
- if err != nil {
- s.fail(w, r, err)
- return
- }
- data.Tokens = tokenRows(tokens)
-
- vd := s.chrome(r)
- vd.Title = "Agent tokens"
- vd.Data = data
- s.render(w, http.StatusOK, "tokens", vd)
-}
-
-// tokenRows turns the service view onto listing lines, formatting the two
-// timestamps here so the template holds no date logic.
-func tokenRows(ts []service.AgentToken) []tokenRow {
- rows := make([]tokenRow, 0, len(ts))
- for _, t := range ts {
- row := tokenRow{
- ID: t.ID,
- Name: t.Name,
- Created: t.Created.UTC().Format(time.RFC3339),
- Active: t.Active(),
- }
- if !row.Active {
- row.Revoked = t.Revoked.UTC().Format(time.RFC3339)
- }
- rows = append(rows, row)
- }
- return rows
+// No principal check. The old page was owner-only because it listed and minted
+// credentials; a redirect exposes nothing but a public origin already in the
+// nav, and tokens.sr.ht authenticates its own page against the same
+// unified-login cookie this service reads. Sending an anonymous browser through
+// meta's login first would only add a round trip to the same destination.
+//
+// A 303 rather than a 301: the destination of this route is an instance
+// configuration value, and a permanent redirect is cached by browsers for far
+// longer than a config key stays true.
+func (s *Server) handleTokens(w http.ResponseWriter, r *http.Request) {
+ if s.tokensOrigin == "" {
+ // An instance with no [tokens.sr.ht] origin has no page to send anybody
+ // to, and inventing one would land the operator on a dead host. It is
+ // also not a state this daemon can serve agents in — service.New refuses
+ // to build the agent plane without that origin — so the page says what is
+ // actually wrong.
+ s.renderError(w, r, http.StatusServiceUnavailable,
+ "agent credentials are issued by tokens.sr.ht, and this instance's config.ini "+
+ "has no [tokens.sr.ht] origin")
+ return
+ }
+ http.Redirect(w, r, s.tokensOrigin+tokensPath, http.StatusSeeOther)
}
M web/tokens_test.go => web/tokens_test.go +59 -146
@@ 3,162 3,75 @@ package web
import (
"net/http"
"net/http/httptest"
- "net/url"
- "strings"
"testing"
- "time"
- "sourcecraft.dev/bigbes/sr-ht-spec/service"
+ "github.com/stretchr/testify/assert"
+ "github.com/stretchr/testify/require"
+ "github.com/vaughan0/go-ini"
)
-// selfOrigin is this instance's origin as testServerWith configures it — the
-// value the cross-site guard accepts.
-const selfOrigin = "https://spec.example"
-
-func TestTokensPageListsWhatExists(t *testing.T) {
- r := newFakeReader()
- revoked := time.Date(2026, 8, 1, 10, 0, 0, 0, time.UTC)
- r.tokens = []service.AgentToken{
- {ID: 2, Name: "claude-code", Created: time.Date(2026, 8, 2, 9, 0, 0, 0, time.UTC)},
- {ID: 1, Name: "old-runner", Created: time.Date(2026, 7, 1, 9, 0, 0, 0, time.UTC), Revoked: &revoked},
- }
- h, _, _ := testServerWith(t, r)
-
- rec := get(t, h, "/tokens", "bigbes")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200; body:\n%s", rec.Code, rec.Body)
- }
- body := rec.Body.String()
- for _, want := range []string{"claude-code", "old-runner", "active", "revoked", "/tokens/2/revoke"} {
- if !strings.Contains(body, want) {
- t.Errorf("the page does not mention %q; body:\n%s", want, body)
- }
- }
- // A revoked token has nothing left to revoke.
- if strings.Contains(body, "/tokens/1/revoke") {
- t.Errorf("the page offers to revoke an already-revoked token")
- }
-}
-
-func TestTokensPageEmpty(t *testing.T) {
- h, _, _ := testServerWith(t, newFakeReader())
- rec := get(t, h, "/tokens", "bigbes")
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200", rec.Code)
- }
- if !strings.Contains(rec.Body.String(), "No tokens yet") {
- t.Errorf("an empty inventory does not say so:\n%s", rec.Body)
- }
-}
-
-// TestTokensPageIsOwnerOnly is the whole point of the page's ACL: an agent is
-// authenticated, and still may not see the inventory of who can write — nor be
-// bounced to a login page it has no way to use.
-func TestTokensPageIsOwnerOnly(t *testing.T) {
- h, _, _ := testServerWith(t, newFakeReader())
-
- if rec := getAgent(t, h, "/tokens", agentTk); rec.Code != http.StatusForbidden {
- t.Errorf("an agent got %d for /tokens, want 403", rec.Code)
- }
- rec := get(t, h, "/tokens", "")
- if rec.Code != http.StatusSeeOther && rec.Code != http.StatusFound {
- t.Errorf("an anonymous browser got %d, want a login redirect", rec.Code)
- }
-}
-
-// TestTokenMintShowsThePlaintextOnce proves the response to the mint carries the
-// value the service returned. It is shown here or nowhere: nothing stores it.
-func TestTokenMintShowsThePlaintextOnce(t *testing.T) {
- r := newFakeReader()
- h, _, _ := testServerWith(t, r)
-
- rec := postForm(t, h, "/tokens", "bigbes", selfOrigin, url.Values{"name": {"claude-code"}})
- if rec.Code != http.StatusOK {
- t.Fatalf("status = %d, want 200; body:\n%s", rec.Code, rec.Body)
- }
- if len(r.issued) != 1 {
- t.Fatalf("the service minted %d tokens, want 1", len(r.issued))
- }
- body := rec.Body.String()
- if !strings.Contains(body, r.issued[0]) {
- t.Errorf("the minted token is not on the page; body:\n%s", body)
- }
- if !strings.Contains(body, "only time it is shown") {
- t.Errorf("the page does not warn that the token is shown once")
- }
-
- // A later view of the page must not carry it: it exists only in the
- // response to the request that minted it.
- if again := get(t, h, "/tokens", "bigbes"); strings.Contains(again.Body.String(), r.issued[0]) {
- t.Errorf("a later page view still shows the plaintext:\n%s", again.Body)
+// /tokens is a signpost now. spec.sr.ht mints no credential of its own, so the
+// page that used to mint, list and revoke one points at the daemon that does.
+func TestTokensRedirectsToTokensSrHt(t *testing.T) {
+ h, _ := testServer(t)
+
+ for name, user := range map[string]string{
+ "owner": "bigbes",
+ "anonymous": "",
+ } {
+ t.Run(name, func(t *testing.T) {
+ rec := get(t, h, "/tokens", user)
+ assert.Equal(t, http.StatusSeeOther, rec.Code, "body: %s", rec.Body)
+ // The external origin, because this is for a browser, and the page
+ // SPEC ch. 7 puts the token UI on.
+ assert.Equal(t, "https://tokens.example/tokens", rec.Header().Get("Location"))
+ })
}
}
-func TestTokenMintRefusesANamelessToken(t *testing.T) {
- r := newFakeReader()
- h, _, _ := testServerWith(t, r)
-
- rec := postForm(t, h, "/tokens", "bigbes", selfOrigin, url.Values{"name": {" "}})
- if rec.Code == http.StatusOK {
- t.Errorf("a blank name was accepted: %d", rec.Code)
- }
- if len(r.issued) != 0 {
- t.Errorf("a token was minted for a blank name: %v", r.issued)
+// The POST routes went with the table behind them: nothing here mints or
+// revokes any more, and a form posted at the old address must not 404 into
+// something that looks like it might have worked.
+func TestTokensAcceptsNoWrites(t *testing.T) {
+ h, _ := testServer(t)
+
+ for _, target := range []string{"/tokens", "/tokens/1/revoke"} {
+ t.Run(target, func(t *testing.T) {
+ req := httptest.NewRequest(http.MethodPost, target, nil)
+ login(req, "bigbes")
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ assert.NotEqual(t, http.StatusOK, rec.Code)
+ assert.NotEqual(t, http.StatusSeeOther, rec.Code)
+ })
}
}
-func TestTokenWritesAreOwnerOnlyAndSameOrigin(t *testing.T) {
- r := newFakeReader()
- r.tokens = []service.AgentToken{{ID: 1, Name: "claude-code", Created: time.Now()}}
- h, _, _ := testServerWith(t, r)
-
- // An agent may not mint, even with a valid bearer token.
- req := httptest.NewRequest(http.MethodPost, "/tokens", strings.NewReader("name=self"))
- req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
- req.Header.Set("Authorization", "Bearer "+agentTk)
- req.Header.Set("Origin", selfOrigin)
+// An instance with no [tokens.sr.ht] section has nowhere to send anybody, and
+// says so instead of redirecting to a URL built out of an empty string.
+func TestTokensWithoutTheSectionSaysSo(t *testing.T) {
+ srv, err := New(Options{
+ Conf: ini.File{
+ "sr.ht": ini.Section{
+ "network-key": testConf.Section("sr.ht")["network-key"],
+ "owner-name": "bigbes",
+ },
+ "webhooks": ini.Section{"private-key": testConf.Section("webhooks")["private-key"]},
+ "spec.sr.ht": ini.Section{"origin": "https://spec.example"},
+ "meta.sr.ht": ini.Section{"origin": "https://meta.example"},
+ },
+ Reader: newFakeReader(),
+ Searcher: &fakeSearcher{},
+ Resolver: testResolver(t),
+ })
+ require.NoError(t, err)
+
+ req := httptest.NewRequest(http.MethodGet, "/tokens", nil)
+ login(req, "bigbes")
rec := httptest.NewRecorder()
- h.ServeHTTP(rec, req)
- if rec.Code != http.StatusForbidden {
- t.Errorf("an agent minting got %d, want 403", rec.Code)
- }
-
- // The owner's own form post from somewhere else is a forgery.
- if rec := postForm(t, h, "/tokens", "bigbes", "https://evil.example",
- url.Values{"name": {"x"}}); rec.Code != http.StatusForbidden {
- t.Errorf("a cross-origin mint got %d, want 403", rec.Code)
- }
- if rec := post(t, h, "/tokens/1/revoke", "bigbes", "https://evil.example"); rec.Code != http.StatusForbidden {
- t.Errorf("a cross-origin revoke got %d, want 403", rec.Code)
- }
- if len(r.issued) != 0 {
- t.Errorf("a refused request still minted: %v", r.issued)
- }
- if !r.tokens[0].Active() {
- t.Errorf("a refused request still revoked the token")
- }
-}
-
-func TestTokenRevokeStampsAndRedirects(t *testing.T) {
- r := newFakeReader()
- r.tokens = []service.AgentToken{{ID: 4, Name: "claude-code", Created: time.Now()}}
- h, _, _ := testServerWith(t, r)
-
- rec := post(t, h, "/tokens/4/revoke", "bigbes", selfOrigin)
- if rec.Code != http.StatusSeeOther {
- t.Fatalf("status = %d, want 303; body:\n%s", rec.Code, rec.Body)
- }
- if got := rec.Header().Get("Location"); got != "/tokens" {
- t.Errorf("Location = %q want /tokens", got)
- }
- if r.tokens[0].Active() {
- t.Errorf("the token is still active after a revoke")
- }
-}
+ srv.Handler().ServeHTTP(rec, req)
-func TestTokenRevokeRejectsAMalformedID(t *testing.T) {
- h, _, _ := testServerWith(t, newFakeReader())
- if rec := post(t, h, "/tokens/abc/revoke", "bigbes", selfOrigin); rec.Code != http.StatusNotFound {
- t.Errorf("status = %d, want 404", rec.Code)
- }
+ assert.Equal(t, http.StatusServiceUnavailable, rec.Code)
+ assert.Empty(t, rec.Header().Get("Location"), "there is no origin to redirect to")
+ assert.Contains(t, rec.Body.String(), "tokens.sr.ht")
}
M web/web_test.go => web/web_test.go +48 -57
@@ 18,7 18,9 @@ import (
"github.com/fernet/fernet-go"
"github.com/vaughan0/go-ini"
+ "sourcecraft.dev/bigbes/sr-ht-core/auth"
"sourcecraft.dev/bigbes/sr-ht-core/crypto"
+ "sourcecraft.dev/bigbes/sr-ht-ecore/bearer"
"sourcecraft.dev/bigbes/sr-ht-spec/authn"
"sourcecraft.dev/bigbes/sr-ht-spec/core"
@@ 44,6 46,9 @@ func TestMain(m *testing.M) {
"webhooks": ini.Section{"private-key": base64.StdEncoding.EncodeToString(seed)},
}
crypto.InitCrypto(testConf)
+ // The agent credential is a signed tokens.sr.ht working token now, so it
+ // cannot be a constant: it is minted here, once the signing key exists.
+ agentTk = agentToken("spec:read")
os.Exit(m.Run())
}
@@ 52,9 57,24 @@ func TestMain(m *testing.M) {
const (
headRev = "1111111111111111111111111111111111111111"
oldRev = "2222222222222222222222222222222222222222"
- agentTk = "test-agent-token"
)
+// agentTk is a live working token for the instance owner, carrying spec:read —
+// what an agent reading through the web UI presents. Set by TestMain.
+var agentTk string
+
+// agentToken mints a signed working token the way tokens.sr.ht does.
+func agentToken(grantString string) string {
+ bt := &auth.BearerToken{
+ Version: auth.TokenVersion,
+ Expires: auth.ToTimestamp(time.Now().Add(time.Hour)),
+ Grants: grantString,
+ ClientID: bearer.TokensClientID,
+ Username: "bigbes",
+ }
+ return bt.Encode()
+}
+
var demoSpace = core.SpaceRef{Owner: "bigbes", Name: "rfcs"}
// headDocs is the space at its approved head. SPEC-0007 links to SPEC-0003, so
@@ 130,15 150,6 @@ type fakeReader struct {
// against a fake that accepts everything.
threads map[int][]*service.Thread
nextThread int
-
- // tokens backs the /tokens page, with ids handed out by nextToken and
- // minted plaintexts recorded in issued so a test can assert the page showed
- // the value the mint returned. Like the thread methods, the three token
- // methods restate service/'s owner-only rule so the fake refuses what the
- // real service refuses.
- tokens []service.AgentToken
- nextToken int
- issued []string
}
func newFakeReader() *fakeReader {
@@ 357,42 368,6 @@ func (f *fakeReader) thread(id int) *service.Thread {
return nil
}
-func (f *fakeReader) ListTokens(_ context.Context, p authn.Principal) ([]service.AgentToken, error) {
- if !p.IsOwner() {
- return nil, fmt.Errorf("%w: %s may not list agent tokens", service.ErrForbidden, p)
- }
- return append([]service.AgentToken(nil), f.tokens...), nil
-}
-
-func (f *fakeReader) IssueToken(_ context.Context, p authn.Principal, name string) (string, service.AgentToken, error) {
- if !p.IsOwner() {
- return "", service.AgentToken{}, fmt.Errorf("%w: %s may not issue agent tokens", service.ErrForbidden, p)
- }
- if strings.TrimSpace(name) == "" {
- return "", service.AgentToken{}, fmt.Errorf("%w: a token needs a name", service.ErrInvalid)
- }
- f.nextToken++
- tok := service.AgentToken{ID: f.nextToken, Name: strings.TrimSpace(name), Created: time.Now()}
- f.tokens = append([]service.AgentToken{tok}, f.tokens...)
- plaintext := fmt.Sprintf("plaintext-%d", tok.ID)
- f.issued = append(f.issued, plaintext)
- return plaintext, tok, nil
-}
-
-func (f *fakeReader) RevokeToken(_ context.Context, p authn.Principal, id int) error {
- if !p.IsOwner() {
- return fmt.Errorf("%w: %s may not revoke agent tokens", service.ErrForbidden, p)
- }
- for i := range f.tokens {
- if f.tokens[i].ID == id {
- now := time.Now()
- f.tokens[i].Revoked = &now
- return nil
- }
- }
- return fmt.Errorf("%w: agent token %d", service.ErrNotFound, id)
-}
-
func (f *fakeReader) Inbox(_ context.Context) ([]service.Proposal, error) {
var out []service.Proposal
for _, p := range f.proposals {
@@ 451,15 426,31 @@ func (s *fakeSearcher) Search(_ context.Context, q search.Query) (search.Results
}, nil
}
-// stubTokenStore knows exactly one live agent token.
-type stubTokenStore struct{}
+// stubUsers resolves the owner an instance token names to a local row.
+type stubUsers struct{}
-func (stubTokenStore) LookupAgentToken(_ context.Context, hash []byte) (authn.AgentToken, error) {
- want := authn.HashToken(agentTk)
- if string(hash) != string(want) {
- return authn.AgentToken{}, authn.ErrUnknownToken
+func (stubUsers) LookupUser(_ context.Context, username string) (authn.InstanceUser, error) {
+ return authn.InstanceUser{ID: 1, Username: username}, nil
+}
+
+// testResolver is the resolver the daemon builds, with the one agent credential
+// plane wired. Its origin is never reached: these tests present stateless
+// tokens, which carry no row id and so skip the revocation round trip.
+func testResolver(t *testing.T) *authn.Resolver {
+ t.Helper()
+ v, err := bearer.New(bearer.Options{
+ Origin: "https://tokens.srht.invalid",
+ ClientID: "spec.sr.ht",
+ NodeID: "web-test",
+ })
+ if err != nil {
+ t.Fatalf("bearer.New: %v", err)
+ }
+ resolver, err := authn.NewResolver("bigbes", authn.WithInstancePlane(v, stubUsers{}))
+ if err != nil {
+ t.Fatalf("NewResolver: %v", err)
}
- return authn.AgentToken{ID: 1, Name: "test", Hash: want}, nil
+ return resolver
}
// testServer wires a Server with a fresh fake reader/searcher behind the same
@@ 490,11 481,11 @@ func testServerWith(t *testing.T, reader *fakeReader) (http.Handler, *fakeReader
"paste.sr.ht": ini.Section{"origin": "https://paste.example"},
"pages.sr.ht": ini.Section{"origin": "https://pages.example"},
"hub.sr.ht": ini.Section{"origin": "https://hub.example"},
+ // /tokens redirects here, and the external origin is the one a browser
+ // can reach.
+ "tokens.sr.ht": ini.Section{"origin": "https://tokens.example"},
}
- resolver, err := authn.NewResolver("bigbes", stubTokenStore{})
- if err != nil {
- t.Fatalf("NewResolver: %v", err)
- }
+ resolver := testResolver(t)
searcher := &fakeSearcher{}
srv, err := New(Options{
Conf: conf,