mirror of
https://github.com/tiennm99/miti99bot.git
synced 2026-07-27 20:19:50 +00:00
refactor(server): drop HTTP /cron route; run crons in-process only
The in-process scheduler is the sole cron trigger on self-host, so the
/cron/{name} HTTP endpoint and its CRON_SHARED_SECRET are dead surface.
Reduce the HTTP server to GET / (health) and remove the cron secret config,
SSM binding, and now-unused timeout.
This commit is contained in:
+4
-19
@@ -143,9 +143,6 @@ func main() {
|
||||
if cfg.BotOwnerID == 0 {
|
||||
log.Warn("OWNER_ID unset; all Private + Protected commands will be denied")
|
||||
}
|
||||
if cfg.CronSecret == "" {
|
||||
log.Warn("CRON_SHARED_SECRET unset; /cron/{name} disabled (404 to all)")
|
||||
}
|
||||
|
||||
deploynotify.Run(rootCtx, deploynotify.Config{
|
||||
Bot: b,
|
||||
@@ -154,23 +151,16 @@ func main() {
|
||||
GitSHA: gitSHA,
|
||||
})
|
||||
|
||||
handler := server.New(server.Config{
|
||||
Bot: b,
|
||||
Registry: reg,
|
||||
CronSecret: cfg.CronSecret,
|
||||
})
|
||||
handler := server.New()
|
||||
|
||||
srv := &http.Server{
|
||||
Addr: ":" + cfg.Port,
|
||||
Handler: handler,
|
||||
ReadHeaderTimeout: 10 * time.Second,
|
||||
ReadTimeout: 30 * time.Second,
|
||||
// 75s = cron handler cap (60s, internal/server/timeouts.go) plus a
|
||||
// 15s margin for response serialization. On Lambda the 30s function
|
||||
// timeout supersedes this; the tighter ceiling matters only for
|
||||
// local non-Lambda runs where a 6-minute slow-loris write was the
|
||||
// previous (over-generous) bound.
|
||||
WriteTimeout: 75 * time.Second,
|
||||
// The only route is GET / (health). It responds instantly, so a tight
|
||||
// write deadline is ample and bounds any slow-loris write.
|
||||
WriteTimeout: 30 * time.Second,
|
||||
IdleTimeout: 120 * time.Second,
|
||||
}
|
||||
|
||||
@@ -267,7 +257,6 @@ func buildProvider(ctx context.Context, cfg config) (storage.Provider, func(), e
|
||||
type config struct {
|
||||
Port string
|
||||
TelegramBotToken string
|
||||
CronSecret string
|
||||
GeminiAPIKey string
|
||||
GoldPriceAPIURL string
|
||||
GoldFXAPIURL string
|
||||
@@ -283,7 +272,6 @@ type config struct {
|
||||
MongoURL string // required when KVProvider=mongodb (Atlas SRV connection string; SECRET — never log)
|
||||
MongoDatabase string // required when KVProvider=mongodb
|
||||
TelegramBotTokenParam string
|
||||
CronSecretParam string
|
||||
GeminiAPIKeyParam string
|
||||
GoldVNAppAPIKeyParam string
|
||||
}
|
||||
@@ -308,7 +296,6 @@ func loadConfig() config {
|
||||
return config{
|
||||
Port: port,
|
||||
TelegramBotToken: envMap["TELEGRAM_BOT_TOKEN"],
|
||||
CronSecret: envMap["CRON_SHARED_SECRET"],
|
||||
GeminiAPIKey: envMap["GEMINI_API_KEY"],
|
||||
GoldPriceAPIURL: envMap["GOLD_PRICE_API_URL"],
|
||||
GoldFXAPIURL: envMap["GOLD_FX_API_URL"],
|
||||
@@ -324,7 +311,6 @@ func loadConfig() config {
|
||||
MongoURL: envMap["MONGO_URL"],
|
||||
MongoDatabase: envMap["MONGO_DATABASE"],
|
||||
TelegramBotTokenParam: strings.TrimSpace(envMap["TELEGRAM_BOT_TOKEN_PARAMETER_NAME"]),
|
||||
CronSecretParam: strings.TrimSpace(envMap["CRON_SHARED_SECRET_PARAMETER_NAME"]),
|
||||
GeminiAPIKeyParam: strings.TrimSpace(envMap["GEMINI_API_KEY_PARAMETER_NAME"]),
|
||||
GoldVNAppAPIKeyParam: strings.TrimSpace(envMap["GOLD_VNAPP_API_KEY_PARAMETER_NAME"]),
|
||||
}
|
||||
@@ -336,7 +322,6 @@ func resolveSSMSecrets(ctx context.Context, cfg *config) error {
|
||||
target *string
|
||||
}{
|
||||
{name: cfg.TelegramBotTokenParam, target: &cfg.TelegramBotToken},
|
||||
{name: cfg.CronSecretParam, target: &cfg.CronSecret},
|
||||
{name: cfg.GeminiAPIKeyParam, target: &cfg.GeminiAPIKey},
|
||||
{name: cfg.GoldVNAppAPIKeyParam, target: &cfg.GoldVNAppAPIKey},
|
||||
}
|
||||
|
||||
@@ -16,17 +16,15 @@ import (
|
||||
"github.com/tiennm99/miti99bot/internal/modules"
|
||||
)
|
||||
|
||||
// cronTimeout caps a single in-process cron fire. Defined locally (rather than
|
||||
// imported from internal/server) to avoid a server→cron layering inversion;
|
||||
// it intentionally matches internal/server's defaultCronTimeout so the
|
||||
// in-process path and the HTTP /cron path share the same 60s budget.
|
||||
// cronTimeout caps a single in-process cron fire so a slow handler cannot
|
||||
// wedge the scheduler. 60s is the budget; long crons must offload and return
|
||||
// fast.
|
||||
const cronTimeout = 60 * time.Second
|
||||
|
||||
// Run starts an in-process scheduler that fires every registered cron with a
|
||||
// non-empty Schedule. Each fire is wrapped with a timeout, panic recovery, and
|
||||
// structured logging — modules.DispatchScheduled provides none of these (it is
|
||||
// only a registry lookup + handler call), so the wrapping lives here, mirroring
|
||||
// the server-package cronHandler.
|
||||
// only a registry lookup + handler call), so the wrapping lives here.
|
||||
//
|
||||
// A malformed schedule string is fatal (returned as an error) so a config typo
|
||||
// fails fast at startup rather than silently never firing. The returned stop
|
||||
|
||||
@@ -6,16 +6,16 @@ import (
|
||||
"fmt"
|
||||
)
|
||||
|
||||
// ErrCronNotFound is returned when /cron/{name} addresses an unregistered cron.
|
||||
// ErrCronNotFound is returned when name addresses an unregistered cron.
|
||||
var ErrCronNotFound = errors.New("cron not found")
|
||||
|
||||
// DispatchScheduled runs the cron registered under name with the per-module
|
||||
// prefixed Deps the registry stored at Build time. Returns ErrCronNotFound if
|
||||
// no module owns that name — EventBridge Scheduler hitting an unknown route is a
|
||||
// configuration bug worth surfacing as a 404 at the HTTP layer.
|
||||
// no module owns that name — a scheduler firing an unknown name is a
|
||||
// configuration bug worth surfacing to the caller.
|
||||
//
|
||||
// The handler runs synchronously in the calling goroutine; ctx propagates
|
||||
// cancellation/timeout from the HTTP request.
|
||||
// the scheduler's per-fire cancellation/timeout.
|
||||
func DispatchScheduled(ctx context.Context, name string, reg *Registry) error {
|
||||
cron, ok := reg.Cron(name)
|
||||
if !ok {
|
||||
|
||||
+6
-107
@@ -1,120 +1,19 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"crypto/subtle"
|
||||
"errors"
|
||||
"net/http"
|
||||
"regexp"
|
||||
"runtime/debug"
|
||||
"strings"
|
||||
import "net/http"
|
||||
|
||||
"github.com/go-telegram/bot"
|
||||
|
||||
"github.com/tiennm99/miti99bot/internal/log"
|
||||
"github.com/tiennm99/miti99bot/internal/modules"
|
||||
)
|
||||
|
||||
// cronNameRe limits cron path segments to a safe alphabet so log injection via
|
||||
// the route is impossible (newlines, ANSI escapes, etc. are rejected at the
|
||||
// router boundary). Same shape as Telegram command names.
|
||||
var cronNameRe = regexp.MustCompile(`^[a-z0-9_]{1,32}$`)
|
||||
|
||||
// cronAuthHeader is the shared-secret header a caller attaches when invoking
|
||||
// /cron/{name} for a manual trigger.
|
||||
const cronAuthHeader = "X-Cron-Token"
|
||||
|
||||
// Config wires the router's runtime dependencies.
|
||||
type Config struct {
|
||||
Bot *bot.Bot
|
||||
Registry *modules.Registry
|
||||
|
||||
// CronSecret protects /cron/{name} against unauthenticated calls; a caller
|
||||
// attaches it as the X-Cron-Token header. Empty means /cron/{name} is fully
|
||||
// disabled (404) — the default on self-host, where the in-process scheduler
|
||||
// (internal/cron) is the sole cron trigger.
|
||||
CronSecret string
|
||||
}
|
||||
|
||||
// New builds the application's HTTP handler. Routes:
|
||||
// New builds the application's HTTP handler. The only route is:
|
||||
//
|
||||
// GET / → health (Coolify container monitor; not publicly routed)
|
||||
// POST /cron/{name} → optional manual cron trigger (shared-secret check)
|
||||
// GET / → health (Coolify container monitor; not publicly routed)
|
||||
//
|
||||
// There is no /webhook route: Telegram updates arrive via long polling
|
||||
// (cmd/server runs b.Start), so the bot needs no public inbound ingress.
|
||||
// There is no /cron route either: crons fire from the in-process scheduler
|
||||
// (internal/cron), so cron has no HTTP surface to expose or protect.
|
||||
// Anything else is 404. All routes pass through LogRequests so every request
|
||||
// emits a structured `req` log line.
|
||||
func New(cfg Config) http.Handler {
|
||||
func New() http.Handler {
|
||||
mux := http.NewServeMux()
|
||||
mux.Handle("/", HealthHandler())
|
||||
mux.Handle("/cron/", cronHandler(cfg.Registry, cfg.CronSecret))
|
||||
return LogRequests(mux)
|
||||
}
|
||||
|
||||
func cronHandler(reg *modules.Registry, secret string) http.HandlerFunc {
|
||||
secretBytes := []byte(secret)
|
||||
cronDisabled := secret == ""
|
||||
return func(w http.ResponseWriter, r *http.Request) {
|
||||
// Rejection paths use bare status codes (no response body) so a
|
||||
// scanner hitting /cron/ can't fingerprint the route from the
|
||||
// response text. Status codes remain distinct for CloudWatch
|
||||
// metric filters; structured log lines carry the reason for
|
||||
// operator triage.
|
||||
if cronDisabled {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
if r.Method != http.MethodPost {
|
||||
log.Warn("cron rejected", "reason", "method", "method", r.Method)
|
||||
w.WriteHeader(http.StatusMethodNotAllowed)
|
||||
return
|
||||
}
|
||||
got := []byte(r.Header.Get(cronAuthHeader))
|
||||
if subtle.ConstantTimeCompare(got, secretBytes) != 1 {
|
||||
log.Warn("cron rejected", "reason", "secret_mismatch")
|
||||
w.WriteHeader(http.StatusUnauthorized)
|
||||
return
|
||||
}
|
||||
|
||||
name := strings.TrimPrefix(r.URL.Path, "/cron/")
|
||||
if !cronNameRe.MatchString(name) {
|
||||
log.Warn("cron rejected", "reason", "bad_name", "name", name)
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
|
||||
log.Info("cron triggered", "route", "/cron", "name", name)
|
||||
ctx, cancel := context.WithTimeout(r.Context(), defaultCronTimeout)
|
||||
defer cancel()
|
||||
|
||||
// Recover panics with cron-name context BEFORE the LogRequests
|
||||
// middleware's safety-net recover sees them — otherwise CloudWatch
|
||||
// would just show "middleware recovered panic" with no clue which
|
||||
// scheduled job blew up. EventBridge sees 500 either way.
|
||||
var dispatchErr error
|
||||
func() {
|
||||
defer func() {
|
||||
if rec := recover(); rec != nil {
|
||||
log.Error("cron handler panic",
|
||||
"route", "/cron",
|
||||
"name", name,
|
||||
"panic", rec,
|
||||
"stack", string(debug.Stack()))
|
||||
dispatchErr = errors.New("cron handler panicked")
|
||||
}
|
||||
}()
|
||||
dispatchErr = modules.DispatchScheduled(ctx, name, reg)
|
||||
}()
|
||||
if dispatchErr != nil {
|
||||
if errors.Is(dispatchErr, modules.ErrCronNotFound) {
|
||||
w.WriteHeader(http.StatusNotFound)
|
||||
return
|
||||
}
|
||||
log.Error("cron failed", "route", "/cron", "name", name, "err", dispatchErr)
|
||||
w.WriteHeader(http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
w.WriteHeader(http.StatusOK)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,27 +1,12 @@
|
||||
package server
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/tiennm99/miti99bot/internal/modules"
|
||||
"github.com/tiennm99/miti99bot/internal/storage"
|
||||
)
|
||||
|
||||
const testCronSecret = "shared-cron-secret"
|
||||
|
||||
func buildRegistry(t *testing.T, factories map[string]modules.Factory, names ...string) *modules.Registry {
|
||||
t.Helper()
|
||||
reg, err := modules.Build(names, factories, storage.NewMemoryProvider(), modules.BuildOptions{})
|
||||
if err != nil {
|
||||
t.Fatalf("modules.Build: %v", err)
|
||||
}
|
||||
return reg
|
||||
}
|
||||
|
||||
func TestHealthHandler_OK(t *testing.T) {
|
||||
rec := httptest.NewRecorder()
|
||||
HealthHandler()(rec, httptest.NewRequest(http.MethodGet, "/", nil))
|
||||
@@ -32,106 +17,3 @@ func TestHealthHandler_OK(t *testing.T) {
|
||||
t.Errorf("body = %q, want contains 'ok'", rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronHandler_DisabledWhenSecretEmpty(t *testing.T) {
|
||||
reg := buildRegistry(t, nil)
|
||||
h := cronHandler(reg, "")
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h(rec, httptest.NewRequest(http.MethodPost, "/cron/anything", nil))
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("status = %d, want 404 (disabled)", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronHandler_RejectsNonPost(t *testing.T) {
|
||||
reg := buildRegistry(t, nil)
|
||||
h := cronHandler(reg, testCronSecret)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h(rec, httptest.NewRequest(http.MethodGet, "/cron/anything", nil))
|
||||
if rec.Code != http.StatusMethodNotAllowed {
|
||||
t.Errorf("status = %d, want 405", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronHandler_RejectsMissingAuth(t *testing.T) {
|
||||
reg := buildRegistry(t, nil)
|
||||
h := cronHandler(reg, testCronSecret)
|
||||
|
||||
rec := httptest.NewRecorder()
|
||||
h(rec, httptest.NewRequest(http.MethodPost, "/cron/anything", nil))
|
||||
if rec.Code != http.StatusUnauthorized {
|
||||
t.Errorf("status = %d, want 401", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronHandler_RejectsInvalidName(t *testing.T) {
|
||||
reg := buildRegistry(t, nil)
|
||||
h := cronHandler(reg, testCronSecret)
|
||||
|
||||
// Use bare names that fail the regex; net/http already %-decodes the
|
||||
// path, so a smuggled \n on the wire would arrive here as a literal byte.
|
||||
cases := map[string]string{
|
||||
"uppercase": "/cron/BadName",
|
||||
"hyphen": "/cron/with-dash",
|
||||
"newline": "/cron/with\nnewline",
|
||||
"empty": "/cron/",
|
||||
"too long": "/cron/abcdefghijklmnopqrstuvwxyzabcdefg", // 33 chars
|
||||
"path nested": "/cron/foo/bar",
|
||||
}
|
||||
for label, path := range cases {
|
||||
t.Run(label, func(t *testing.T) {
|
||||
req := httptest.NewRequest(http.MethodPost, "/cron/x", nil)
|
||||
req.URL.Path = path // bypass NewRequest's URL parser
|
||||
req.Header.Set(cronAuthHeader, testCronSecret)
|
||||
rec := httptest.NewRecorder()
|
||||
h(rec, req)
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("path %q: status = %d, want 404", path, rec.Code)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronHandler_UnknownNameReturns404(t *testing.T) {
|
||||
reg := buildRegistry(t, nil)
|
||||
h := cronHandler(reg, testCronSecret)
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/cron/missing", nil)
|
||||
req.Header.Set(cronAuthHeader, testCronSecret)
|
||||
rec := httptest.NewRecorder()
|
||||
h(rec, req)
|
||||
if rec.Code != http.StatusNotFound {
|
||||
t.Errorf("status = %d, want 404", rec.Code)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCronHandler_RunsRegisteredCron(t *testing.T) {
|
||||
called := false
|
||||
factories := map[string]modules.Factory{
|
||||
"alpha": func(_ modules.Deps) modules.Module {
|
||||
return modules.Module{Crons: []modules.Cron{{
|
||||
Name: "tick",
|
||||
Handler: func(_ context.Context, _ modules.Deps) error {
|
||||
called = true
|
||||
return nil
|
||||
},
|
||||
}}}
|
||||
},
|
||||
}
|
||||
reg := buildRegistry(t, factories, "alpha")
|
||||
h := cronHandler(reg, testCronSecret)
|
||||
|
||||
req := httptest.NewRequest(http.MethodPost, "/cron/tick", nil)
|
||||
req.Header.Set(cronAuthHeader, testCronSecret)
|
||||
rec := httptest.NewRecorder()
|
||||
h(rec, req)
|
||||
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Errorf("status = %d, want 200", rec.Code)
|
||||
}
|
||||
if !called {
|
||||
t.Error("cron handler not invoked")
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,9 +0,0 @@
|
||||
package server
|
||||
|
||||
import "time"
|
||||
|
||||
// defaultCronTimeout caps a single /cron/{name} invocation. Lambda free
|
||||
// tier runs at most 1 instance, so a long cron serializes all other crons
|
||||
// behind it and amplifies any DoS via the cron route. 60s is the budget; long
|
||||
// crons must publish to PubSub and exit fast.
|
||||
const defaultCronTimeout = 60 * time.Second
|
||||
Reference in New Issue
Block a user