Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 21 additions & 1 deletion cmd/octobus/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"octobus/internal/admin"
"octobus/internal/cli"
"octobus/internal/daemonlog"
"octobus/internal/domain"
"octobus/internal/packageimport"
"octobus/internal/protocol"
"octobus/internal/server"
Expand Down Expand Up @@ -125,7 +126,10 @@ func serve(opts serveOptions) error {
if err := startupInventory(ctx, logger, st); err != nil {
return err
}
adminServer := &admin.Server{Store: st, Importer: &packageimport.Importer{DataDir: dataDir, Store: st}, Supervisor: sup, Gateway: gateway, AccessLogPath: filepath.Join(dataDir, accesslog.FileName), Logger: logger}
if err := initializeAdminAuth(ctx, st); err != nil {
return fmt.Errorf("initialize admin authentication: %w", err)
}
adminServer := &admin.Server{Store: st, Importer: &packageimport.Importer{DataDir: dataDir, Store: st}, Supervisor: sup, Gateway: gateway, AccessLogPath: filepath.Join(dataDir, accesslog.FileName), Logger: logger, RequireAdminToken: true}
grpcServer := protocol.GRPCServer(gateway)
publicServer := admin.NewHTTPServer(opts.addr, h2c.NewHandler(server.CombinedHandler(adminServer.Handler(), grpcServer, gateway), &http2.Server{}))
publicListener, err := net.Listen("tcp", opts.addr)
Expand Down Expand Up @@ -173,6 +177,22 @@ func shutdownSupervisor(logger *slog.Logger, sup *supervisor.Supervisor) {
}
}

func initializeAdminAuth(ctx context.Context, st *store.Store) error {
requires, err := st.AdminRequiresToken(ctx)
if err != nil {
return err
}
if requires {
return nil
}
secret := os.Getenv("OCTOBUS_BOOTSTRAP_ADMIN_TOKEN")
if secret == "" {
return nil
}
_, err = st.AddAdminToken(ctx, domain.AdminToken{ID: "bootstrap-admin", Name: "Bootstrap admin"}, secret)
return err
}
Comment thread
monkeyscan[bot] marked this conversation as resolved.

func logStartupInventory(ctx context.Context, logger *slog.Logger, st *store.Store) error {
logger = daemonlog.OrNop(logger)
capsets, err := st.ListCapsets(ctx)
Expand Down
27 changes: 27 additions & 0 deletions cmd/octobus/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,33 @@ func TestMain(m *testing.M) {
os.Exit(m.Run())
}

func TestInitializeAdminAuthBootstrapsOnlyWhenStoreIsEmpty(t *testing.T) {
st, err := store.Open(filepath.Join(t.TempDir(), "octobus.db"))
if err != nil {
t.Fatal(err)
}
defer st.Close()

t.Setenv("OCTOBUS_BOOTSTRAP_ADMIN_TOKEN", "bootstrap-secret")
if err := initializeAdminAuth(context.Background(), st); err != nil {
t.Fatal(err)
}
requires, err := st.AdminRequiresToken(context.Background())
if err != nil {
t.Fatal(err)
}
if !requires {
t.Fatal("bootstrap token was not persisted")
}
ok, err := st.VerifyAdminToken(context.Background(), "bootstrap-secret")
if err != nil {
t.Fatal(err)
}
if !ok {
t.Fatal("bootstrap token was not usable")
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bootstrap 测试仅覆盖快乐路径,缺少对『已要求 token』与『未设置环境变量』负分支的覆盖

新增测试 TestInitializeAdminAuthBootstrapsOnlyWhenStoreIsEmpty 的命名声明了『仅当 store 为空时自举』,但实际只覆盖了『store 为空 + 环境变量已设置』的快乐路径,没有覆盖两个关键负分支:a) store 已要求 token(已有 token)时设置环境变量应被忽略且不应报错/重复创建同名 token;b) store 为空但环境变量未设置时应 no-op(而这一路径正是导致 admin API 被静默锁死的主因)。鉴于该逻辑的回归风险较高(错误的自举可能导致重复 token、启动失败或控制面锁死),建议补充这两个分支的用例以锁定预期行为。

Problem code:

Changed code at cmd/octobus/main_test.go:38-63

Recommendation:
为 initializeAdminAuth 补充『store 已有 token 且设置了环境变量』(验证不会重复创建、不会返回错误)与『store 为空且未设置环境变量』(验证 no-op 后 AdminRequiresToken 仍为 false)两个用例,并在 serve 集成层验证默认启动后非 status 的 admin 端点返回 401。


func TestRootAddrFlagOverridesAdminCommands(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.Method != http.MethodGet || r.URL.Path != "/admin/v1/status" {
Expand Down
17 changes: 13 additions & 4 deletions internal/admin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ type Server struct {
Gateway *protocol.Gateway
AccessLogPath string
Logger *slog.Logger

// RequireAdminToken is enabled by the daemon for production control-plane
// handlers. It is explicit so lightweight in-process test servers can keep
// using the handler without provisioning a token store.
RequireAdminToken bool
}

type serviceImporter interface {
Expand Down Expand Up @@ -147,10 +152,14 @@ func (s *Server) adminTokenMiddleware(next echo.HandlerFunc) echo.HandlerFunc {
if c.Request().URL.Path == "/admin/v1/status" {
return next(c)
}
requires, err := s.Store.AdminRequiresToken(c.Request().Context())
if err != nil {
writeError(c.Response(), http.StatusInternalServerError, err.Error())
return nil
requires := s.RequireAdminToken
if !requires {
var err error
requires, err = s.Store.AdminRequiresToken(c.Request().Context())
if err != nil {
writeError(c.Response(), http.StatusInternalServerError, err.Error())
return nil
}
}
if !requires {
return next(c)
Expand Down
16 changes: 16 additions & 0 deletions internal/admin/admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -747,6 +747,22 @@ func TestAdminServiceImportMultipartRecursiveAggregateAndValidation(t *testing.T
})
}

func TestAdminTokenIsRequiredWhenDaemonEnablesControlPlaneAuth(t *testing.T) {
st, err := store.Open(filepath.Join(t.TempDir(), "octobus.db"))
if err != nil {
t.Fatal(err)
}
defer st.Close()

srv := &Server{Store: st, RequireAdminToken: true}
req := httptest.NewRequest(http.MethodGet, "/admin/v1/services", nil)
w := httptest.NewRecorder()
srv.Handler().ServeHTTP(w, req)
if w.Code != http.StatusUnauthorized {
t.Fatalf("status without admin token = %d, want %d", w.Code, http.StatusUnauthorized)
}
}

func TestAdminServiceImportMultipartRequiresAdminToken(t *testing.T) {
ctx := context.Background()
dataDir := t.TempDir()
Expand Down
Loading