Skip to content

Commit e0524c0

Browse files
Bowl42claude
andauthored
fix(bedrock): expose bedrock-models under self-service /api/providers (#534)
* fix(bedrock): expose bedrock-models under self-service /api/providers path The admin handler registered /api/admin/providers/{id}/bedrock-models, but the frontend's default axios baseURL is /api, so every page load from a non-admin deployment hits /api/providers/{id}/bedrock-models — which the self_service handler did not route, yielding a 404. Reported against production at maxx.gemini.skr.rocks after #533 merged. Fix: add the 4-segment "providers/{id}/bedrock-models" case to self_service.go and extract the shared GET/POST logic into serveBedrockDiscoveredModels so both handlers call the same code path instead of diverging. Both /api/admin/… and /api/… now return the identical payload (verified locally with a real Bedrock provider: 200 OK + 14 discovered models on both endpoints). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bedrock): gate self-service POST force-refresh behind admin role Both Claude and CodeRabbit round-1 reviewers flagged the same MAJOR on PR #534: the new /api/providers/{id}/bedrock-models self-service route accepted POST without an admin check, so any authenticated tenant member could hammer force-refresh. The POST path bypasses the in-process TTL and Invalidate() rate-limit and calls ListInferenceProfiles + ListFoundationModels against the provider's AWS account — a cheap amplifier for burning the tenant's AWS API quota. The admin path was implicitly admin-only via its /api/admin/ URL; the self-service mirror inherited no such boundary. Gate POST behind h.requireAdmin so only the admin UI's refresh button reaches it; GET stays available to any authenticated member (same access posture as the providers list itself). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bedrock): wire discovery persistence for desktop startup path Codex round-1 [P2]: bedrock.SetDiscoveryRepository was only called from cmd/maxx/main.go, but the desktop/Wails launcher initializes adapters through core.InitializeServerComponents and never invoked the setter. The persisted Bedrock catalog was a no-op on that path — every desktop restart paid the full ~1-5s ListInferenceProfiles + ListFoundationModels cold-start on the first Bedrock request. Move the wire-up into core.InitializeServerComponents (alongside cooldown persistence) so both startup paths get it: - NewBedrockDiscoveryRepository constructed in InitializeDatabase - Added to DatabaseRepos.BedrockDiscoveryRepo - bedrock.SetDiscoveryRepository called in InitializeServerComponents cmd/maxx/main.go drops the duplicate call and the named bedrock import (back to `_` side-effect registration). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bedrock): hide Refresh button for non-admin viewers Codex round-2 [P2]: the admin-gate added to the self-service POST route gives non-admin tenant members a 403 when they click the Refresh button — the UI still advertised the action even though the backend would reject it. Hide the button when user.role !== 'admin'. Non-admin members can still see the discovery catalog (GET is unrestricted); only the force-refresh action is admin-only, consistent with the backend policy. Admin users see the button unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(bedrock): restore CLI wiring + gate lazy-refresh on non-admin GET Codex round-4 flagged two P2 issues on the rebased branch: 1. Moving bedrock.SetDiscoveryRepository into core.InitializeServerComponents dropped it from the CLI startup — cmd/maxx/main.go doesn't route through core.InitializeServerComponents (only the desktop launcher does). Result: the server binary stopped wiring the repo at all, so every restart paid the full AWS round-trip again. Restore the call in main.go alongside the one in core/database.go; the setter is idempotent so both paths are safe. 2. DiscoveredModels's lazy TTL refresh meant a non-admin tenant member could still trigger ListInferenceProfiles just by opening the provider page after the 30-min window. The POST admin-gate blocked the hammer path but not this slow-drip one. Added an allowLazyRefresh parameter and wired self-service GET to h.isAdmin(r), so non-admin reads return the in-memory snapshot only. Admin GET and POST retain the refresh path. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 11f3cee commit e0524c0

6 files changed

Lines changed: 99 additions & 30 deletions

File tree

cmd/maxx/main.go

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,12 @@ func main() {
121121
inviteCodeRepo := sqlite.NewInviteCodeRepository(db)
122122
inviteCodeUsageRepo := sqlite.NewInviteCodeUsageRepository(db)
123123

124-
// Wire Bedrock discovery persistence: each BedrockAdapter will rehydrate
125-
// its short-name → inference-profile catalog from SQLite at startup,
126-
// avoiding the ~1-5s ListInferenceProfiles round-trip on the first
127-
// request after a restart.
124+
// Wire Bedrock discovery persistence. The CLI entry point does not
125+
// go through core.InitializeServerComponents (the desktop launcher
126+
// does — the desktop path gets the same call from core/database.go),
127+
// so it needs its own setter call; otherwise the server process
128+
// leaves the repo unset and the first Bedrock request after every
129+
// restart pays the full AWS discovery round-trip.
128130
bedrock.SetDiscoveryRepository(sqlite.NewBedrockDiscoveryRepository(db))
129131

130132
// Initialize cooldown manager with database persistence

internal/adapter/provider/bedrock/adapter.go

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -126,13 +126,21 @@ func (a *BedrockAdapter) RefreshDiscoveredModels(ctx context.Context) (Discovere
126126
defer cancel()
127127
refreshErr = a.discoverer.ForceRefresh(lookupCtx)
128128
}
129-
return a.DiscoveredModels(ctx), refreshErr
129+
// Refresh path already forced a fetch; pass false so DiscoveredModels
130+
// doesn't do a redundant second Lookup.
131+
return a.DiscoveredModels(ctx, false), refreshErr
130132
}
131133

132-
// DiscoveredModels triggers a discovery refresh (subject to the normal
133-
// TTL) and returns the current catalog. Uses an isolated background
134-
// context so admin UI polling can't poison the shared cache.
135-
func (a *BedrockAdapter) DiscoveredModels(ctx context.Context) DiscoveredModelsResult {
134+
// DiscoveredModels returns the current catalog. When allowRefresh is
135+
// true, a TTL-expiry Lookup is triggered so the caller gets a fresh
136+
// list if the cache is stale; when false, the in-memory snapshot is
137+
// returned as-is. Admin GETs pass true (they're fine paying an
138+
// occasional AWS round-trip); non-admin GETs from the self-service
139+
// surface pass false, so an anonymous poller cannot wait out the TTL
140+
// to force a ListInferenceProfiles call. Uses an isolated background
141+
// context on the refresh path so admin UI polling can't poison the
142+
// shared cache if the client disconnects.
143+
func (a *BedrockAdapter) DiscoveredModels(ctx context.Context, allowRefresh bool) DiscoveredModelsResult {
136144
region := DefaultRegion
137145
if a.provider != nil && a.provider.Config != nil && a.provider.Config.Bedrock != nil && a.provider.Config.Bedrock.Region != "" {
138146
region = a.provider.Config.Bedrock.Region
@@ -142,11 +150,13 @@ func (a *BedrockAdapter) DiscoveredModels(ctx context.Context) DiscoveredModelsR
142150
return result
143151
}
144152

145-
lookupCtx, cancel := context.WithTimeout(ctx, discoveryLookupTimeout)
146-
defer cancel()
147-
// Force a refresh if the cache is stale. Lookup's argument is a
148-
// miss-on-purpose key — we only want the side effect.
149-
_, _ = a.discoverer.Lookup(lookupCtx, "__admin_refresh__")
153+
if allowRefresh {
154+
lookupCtx, cancel := context.WithTimeout(ctx, discoveryLookupTimeout)
155+
defer cancel()
156+
// Force a refresh if the cache is stale. Lookup's argument is a
157+
// miss-on-purpose key — we only want the side effect.
158+
_, _ = a.discoverer.Lookup(lookupCtx, "__admin_refresh__")
159+
}
150160

151161
entries := a.discoverer.Entries()
152162
result.Available = a.discoverer.Available()

internal/core/database.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import (
88
"time"
99

1010
"github.com/awsl-project/maxx/internal/adapter/client"
11-
_ "github.com/awsl-project/maxx/internal/adapter/provider/bedrock" // Register bedrock adapter
11+
"github.com/awsl-project/maxx/internal/adapter/provider/bedrock"
1212
_ "github.com/awsl-project/maxx/internal/adapter/provider/claude" // Register claude adapter
1313
_ "github.com/awsl-project/maxx/internal/adapter/provider/codex"
1414
_ "github.com/awsl-project/maxx/internal/adapter/provider/custom"
@@ -54,6 +54,7 @@ type DatabaseRepos struct {
5454
CodexQuotaRepo repository.CodexQuotaRepository
5555
CooldownRepo repository.CooldownRepository
5656
FailureCountRepo repository.FailureCountRepository
57+
BedrockDiscoveryRepo repository.BedrockDiscoveryRepository
5758
CachedProviderRepo *cached.ProviderRepository
5859
CachedRouteRepo *cached.RouteRepository
5960
CachedRetryConfigRepo *cached.RetryConfigRepository
@@ -130,6 +131,7 @@ func InitializeDatabase(config *DatabaseConfig) (*DatabaseRepos, error) {
130131
codexQuotaRepo := sqlite.NewCodexQuotaRepository(db)
131132
cooldownRepo := sqlite.NewCooldownRepository(db)
132133
failureCountRepo := sqlite.NewFailureCountRepository(db)
134+
bedrockDiscoveryRepo := sqlite.NewBedrockDiscoveryRepository(db)
133135
apiTokenRepo := sqlite.NewAPITokenRepository(db)
134136
modelMappingRepo := sqlite.NewModelMappingRepository(db)
135137
usageStatsRepo := sqlite.NewUsageStatsRepository(db)
@@ -166,6 +168,7 @@ func InitializeDatabase(config *DatabaseConfig) (*DatabaseRepos, error) {
166168
CodexQuotaRepo: codexQuotaRepo,
167169
CooldownRepo: cooldownRepo,
168170
FailureCountRepo: failureCountRepo,
171+
BedrockDiscoveryRepo: bedrockDiscoveryRepo,
169172
CachedProviderRepo: cachedProviderRepo,
170173
CachedRouteRepo: cachedRouteRepo,
171174
CachedRetryConfigRepo: cachedRetryConfigRepo,
@@ -205,6 +208,14 @@ func InitializeServerComponents(
205208
log.Printf("[Core] Warning: Failed to load cooldowns from database: %v", err)
206209
}
207210

211+
// Wire Bedrock discovery persistence here (not in cmd/maxx/main.go)
212+
// so both the CLI entrypoint and the desktop/Wails launcher — which
213+
// shares this InitializeServerComponents path — benefit from the
214+
// rehydrated catalog. Without this, desktop-mode restarts would pay
215+
// the ~1-5s ListInferenceProfiles cold-start on the first Bedrock
216+
// request even though the rows are on disk.
217+
bedrock.SetDiscoveryRepository(repos.BedrockDiscoveryRepo)
218+
208219
log.Printf("[Core] Marking stale requests as failed")
209220
if count, err := repos.ProxyRequestRepo.MarkStaleAsFailed(instanceID); err != nil {
210221
log.Printf("[Core] Warning: Failed to mark stale requests: %v", err)

internal/handler/admin.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -252,15 +252,29 @@ func (h *AdminHandler) handleProviders(w http.ResponseWriter, r *http.Request, i
252252
// table. Available=false means discovery hasn't succeeded (typically
253253
// missing bedrock:ListInferenceProfiles IAM permission).
254254
func (h *AdminHandler) handleBedrockDiscoveredModels(w http.ResponseWriter, r *http.Request, id uint64) {
255-
// GET returns the current catalog (triggers a lazy refresh on TTL
256-
// expiry). POST forces an immediate fetch bypassing the TTL and the
257-
// Invalidate() rate-limit — used by the admin UI's refresh button.
255+
// Admin surface: callers are always admin, so GET may trigger a
256+
// lazy AWS refresh on TTL expiry.
257+
serveBedrockDiscoveredModels(h.svc, w, r, id, true)
258+
}
259+
260+
// serveBedrockDiscoveredModels is the shared GET/POST handler for the
261+
// Bedrock discovery catalog surface, used by both the admin
262+
// (/api/admin/providers/{id}/bedrock-models) and self-service
263+
// (/api/providers/{id}/bedrock-models) paths. The self-service path is
264+
// what the frontend's default axios baseURL of /api actually hits —
265+
// without this shared handler, the admin-only registration used to 404
266+
// under non-admin deployments.
267+
//
268+
// GET returns the current catalog (triggers a lazy refresh on TTL
269+
// expiry). POST forces an immediate fetch bypassing the TTL and the
270+
// Invalidate() rate-limit — used by the admin UI's refresh button.
271+
func serveBedrockDiscoveredModels(svc *service.AdminService, w http.ResponseWriter, r *http.Request, id uint64, allowLazyRefresh bool) {
258272
if r.Method != http.MethodGet && r.Method != http.MethodPost {
259273
writeJSON(w, http.StatusMethodNotAllowed, map[string]string{"error": "method not allowed"})
260274
return
261275
}
262276
tenantID := maxxctx.GetTenantID(r.Context())
263-
p, err := h.svc.GetProvider(tenantID, id)
277+
p, err := svc.GetProvider(tenantID, id)
264278
if err != nil {
265279
writeJSON(w, http.StatusNotFound, map[string]string{"error": "provider not found"})
266280
return
@@ -269,7 +283,7 @@ func (h *AdminHandler) handleBedrockDiscoveredModels(w http.ResponseWriter, r *h
269283
writeJSON(w, http.StatusBadRequest, map[string]string{"error": "provider is not a bedrock provider"})
270284
return
271285
}
272-
adapter, ok := h.svc.GetProviderAdapter(id)
286+
adapter, ok := svc.GetProviderAdapter(id)
273287
if !ok {
274288
writeJSON(w, http.StatusServiceUnavailable, map[string]string{"error": "adapter not initialized"})
275289
return
@@ -297,7 +311,7 @@ func (h *AdminHandler) handleBedrockDiscoveredModels(w http.ResponseWriter, r *h
297311
})
298312
return
299313
}
300-
writeJSON(w, http.StatusOK, bedrockA.DiscoveredModels(r.Context()))
314+
writeJSON(w, http.StatusOK, bedrockA.DiscoveredModels(r.Context(), allowLazyRefresh))
301315
}
302316

303317
// handleProvidersExport exports all providers as JSON

internal/handler/self_service.go

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,30 @@ func (h *SelfServiceHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
7474
return
7575
}
7676
h.handleProviders(w, r, id)
77+
case len(parts) == 4 && parts[3] == "bedrock-models":
78+
// Mirror the admin endpoint (/api/admin/providers/{id}/bedrock-models)
79+
// under /api/providers/{id}/bedrock-models so the frontend's
80+
// default axios baseURL (/api) can read the discovery catalog
81+
// without talking to the admin-only surface.
82+
//
83+
// GET is readable by any authenticated tenant member (same
84+
// access posture as the providers list). POST forces a fresh
85+
// ListInferenceProfiles + ListFoundationModels round-trip,
86+
// bypassing the in-process TTL and Invalidate() rate-limit —
87+
// gated on admin so a non-privileged member can't hammer it
88+
// and burn the provider's AWS API quota.
89+
id, ok := parseSelfServiceID(w, "provider", parts[2])
90+
if !ok {
91+
return
92+
}
93+
if r.Method == http.MethodPost && !h.requireAdmin(w, r) {
94+
return
95+
}
96+
// Non-admin GET must not be able to trigger a ListInferenceProfiles
97+
// refresh by polling past the TTL window — pass the caller's
98+
// admin status through so DiscoveredModels only lazy-refreshes
99+
// when an admin is on the other end.
100+
serveBedrockDiscoveredModels(h.svc, w, r, id, h.isAdmin(r))
77101
default:
78102
writeJSON(w, http.StatusNotFound, map[string]string{"error": "not found"})
79103
}

web/src/pages/providers/components/bedrock-provider-view.tsx

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { useTranslation } from 'react-i18next';
1717
import type { BedrockDiscoveredModelsResult, Provider } from '@/lib/transport';
1818
import { getTransport } from '@/lib/transport';
1919
import { useUpdateProvider } from '@/hooks/queries';
20+
import { useAuth } from '@/lib/auth-context';
2021
import { Button, Switch } from '@/components/ui';
2122
import { PageHeader } from '@/components/layout/page-header';
2223
import { BEDROCK_COLOR } from '../types';
@@ -31,6 +32,11 @@ interface BedrockProviderViewProps {
3132
export function BedrockProviderView({ provider, onDelete, onClose }: BedrockProviderViewProps) {
3233
const { t } = useTranslation();
3334
const updateProvider = useUpdateProvider();
35+
const { user } = useAuth();
36+
// Refresh triggers a POST that the backend gates behind admin role —
37+
// non-admin tenant members would get a 403. Hide the button for them
38+
// rather than let the UI advertise an action that will fail.
39+
const isAdmin = user?.role === 'admin';
3440
const config = provider.config?.bedrock;
3541

3642
const [showSecret, setShowSecret] = useState(false);
@@ -239,15 +245,17 @@ export function BedrockProviderView({ provider, onDelete, onClose }: BedrockProv
239245
</span>
240246
)}
241247
</div>
242-
<Button
243-
variant="ghost"
244-
size="sm"
245-
onClick={refreshDiscoveredModels}
246-
disabled={discoveryLoading}
247-
>
248-
<RefreshCw size={14} className={discoveryLoading ? 'animate-spin' : ''} />
249-
{t('common.refresh', 'Refresh')}
250-
</Button>
248+
{isAdmin && (
249+
<Button
250+
variant="ghost"
251+
size="sm"
252+
onClick={refreshDiscoveredModels}
253+
disabled={discoveryLoading}
254+
>
255+
<RefreshCw size={14} className={discoveryLoading ? 'animate-spin' : ''} />
256+
{t('common.refresh', 'Refresh')}
257+
</Button>
258+
)}
251259
</div>
252260
<p className="text-xs text-muted-foreground">
253261
{t(

0 commit comments

Comments
 (0)