Backlog: close four P1 findings + the OIDC account-shadowing bypass - #293
Merged
Conversation
Five issues from the Fable review backlog — the four `priority: high` findings plus the OIDC medium in the same cluster. **#269 — OIDC role-map bypass (security).** `_role` fell back to `Role(group.upper())` for every claim value even with a `role_map` configured. Authentik/Okta default `role_claim` to `groups`, so a pre-existing directory group named `Admin` (VPN, Jira, anything) provisioned an Iceberg ADMIN on first login. The name-is-role fallback is now restricted to an *app-roles* claim (`roles` — values the operator defines for this app) with no `role_map` set, which is the legacy single-Entra flow it was built for. A directory `groups` claim always requires an explicit map entry. **#268 — OIDC login routes unthrottled (security).** The `auth-oidc` policy listed only the legacy alias paths, so every provider login through `/auth/oidc/{provider}/{login,callback}` was uncapped — an unauthenticated callback flood wrote an audit row plus a SIEM emit per request. Added a pattern for the parametrised pair; all paths bill one per-IP bucket. **#270 — openai-compatible AI backend (security).** Validation only required a non-empty base URL, and `_complete` attached the env API key as a Bearer header to whatever it resolved to, with no scheme or private-host check. Pinned it to a new `ICEBERG_AI_OPENAI_COMPATIBLE_BASE_URL` operator env value exactly as `ollama` already was: env is the trust anchor, the DB row may only match it, an unset pin refuses the backend. The check now also runs *inside* the backend at call time — the last point before an API key and TLP-gated content leave the process — so it holds for a directly written row, not just an admin-form save. **#276 — OIDC account shadowing (security).** The multi-provider rewrite kept the unbound-email-owner guard on identity creation but dropped it from the update path, so self-service email change at the IdP was a two-step bypass of the admin-must-link policy. Restored on update (and on legacy adoption); bound co-owners stay allowed, which is the designed cross-provider case. **#271 — silent stale-write in the report editor (bug).** With the "Save draft" button gone, autosave is the only save path and treated an optimistic-lock 409 like a network blip: the same stale version was re-posted forever, so a second writer's work was discarded with nothing but an amber "Unsaved changes". Notebook access is role-wide (#65), so that is a supported scenario. The fetch-path save now returns the current version with the 409; the client stops the autosave loop, flips the chip to a conflict reading, and shows a banner with the two explicit ways out — discard and reload, or overwrite with the returned version. A 409 with no version (a published, immutable product) offers reload only. Tests: OIDC group-name escalation, role-map-disables-fallback, app-roles back-compat; parametrised-route throttling and shared bucket; the AI env pin at validation and at runtime (including a link-local host) plus a drift guard between the two pin declarations; email-change shadowing and the bound-co-owner case; and the editor conflict end to end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LQpYwRbt1YhpTXPb46MR5v
This was referenced Aug 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Five issues from the Fable review backlog — the four
priority: highfindings plus the OIDC medium in the same cluster.
#269 — OIDC role-map bypass (security).
_rolefell back toRole(group.upper())for every claim value even with arole_mapconfigured. Authentik/Okta default
role_claimtogroups, so apre-existing directory group named
Admin(VPN, Jira, anything)provisioned an Iceberg ADMIN on first login. The name-is-role fallback is
now restricted to an app-roles claim (
roles— values the operatordefines for this app) with no
role_mapset, which is the legacysingle-Entra flow it was built for. A directory
groupsclaim alwaysrequires an explicit map entry.
#268 — OIDC login routes unthrottled (security). The
auth-oidcpolicy listed only the legacy alias paths, so every provider login through
/auth/oidc/{provider}/{login,callback}was uncapped — an unauthenticatedcallback flood wrote an audit row plus a SIEM emit per request. Added a
pattern for the parametrised pair; all paths bill one per-IP bucket.
#270 — openai-compatible AI backend (security). Validation only
required a non-empty base URL, and
_completeattached the env API key asa Bearer header to whatever it resolved to, with no scheme or private-host
check. Pinned it to a new
ICEBERG_AI_OPENAI_COMPATIBLE_BASE_URLoperatorenv value exactly as
ollamaalready was: env is the trust anchor, the DBrow may only match it, an unset pin refuses the backend. The check now
also runs inside the backend at call time — the last point before an API
key and TLP-gated content leave the process — so it holds for a directly
written row, not just an admin-form save.
#276 — OIDC account shadowing (security). The multi-provider rewrite
kept the unbound-email-owner guard on identity creation but dropped it from
the update path, so self-service email change at the IdP was a two-step
bypass of the admin-must-link policy. Restored on update (and on legacy
adoption); bound co-owners stay allowed, which is the designed
cross-provider case.
#271 — silent stale-write in the report editor (bug). With the
"Save draft" button gone, autosave is the only save path and treated an
optimistic-lock 409 like a network blip: the same stale version was
re-posted forever, so a second writer's work was discarded with nothing
but an amber "Unsaved changes". Notebook access is role-wide (#65), so
that is a supported scenario. The fetch-path save now returns the current
version with the 409; the client stops the autosave loop, flips the chip
to a conflict reading, and shows a banner with the two explicit ways out —
discard and reload, or overwrite with the returned version. A 409 with no
version (a published, immutable product) offers reload only.
Tests: OIDC group-name escalation, role-map-disables-fallback, app-roles
back-compat; parametrised-route throttling and shared bucket; the AI env
pin at validation and at runtime (including a link-local host) plus a
drift guard between the two pin declarations; email-change shadowing and
the bound-co-owner case; and the editor conflict end to end.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01LQpYwRbt1YhpTXPb46MR5v