Skip to content

fix(OPENFRAM-005-11): CU-86akj32d8 dibble IDP seeder writes directly to shared MySQL without tenant scoping guard - #173

Draft
flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/openfram-005-11-ce459428-1446a072
Draft

flamingo[bot] wants to merge 1 commit into
mainfrom
ai-fix/openfram-005-11-ce459428-1446a072

Conversation

@flamingo

@flamingo flamingo Bot commented Sep 14, 2026

Copy link
Copy Markdown

Closes findings from rule OPENFRAM-005-11 — dibble IDP seeder writes directly to shared MySQL without tenant scoping guard.

Draft — this is a starting point, not a finished change. The fix required judgment, so read it before trusting it.

# Fix confidence Finding Location
1 🔴 30 low — review closely dibble IDP seeder writes directly to shared MySQL without tenant scoping guard tools/dibble/pkg/seed/idp.go:59

What changed — and what was deliberately left — is explained per finding as inline review comments on the lines each finding touched.


Run: https://product-hub.flamingo.so/admin/code-review
Run id: 1446a072-096e-4294-8082-c7cadffe76de

Merging this PR is recorded as acceptance of the rule that produced it;
closing it unmerged is recorded as rejection. Both feed rule health, so
closing a wrong suggestion is useful rather than merely tidy.

ClickUp task: CU-86akj32d8 FleetMDM tenant scoping and IDP hardening (1 PRs)

@flamingo flamingo Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 What this fix changed, finding by finding

1 finding(s) fixed in this draft — 1 explained inline on the diff; 1 low-confidence hunk(s) need close review before merging.

// list endpoints are filtered down to this tenant scope (via inOrganization)
// before any mutation, so this seeder cannot attach IDP identities across
// tenant boundaries in a shared multi-tenant deployment.
func IDP(ctx context.Context, c Client, log Logger, opt IDPOptions) Result {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🦩 🔴 dibble IDP seeder writes directly to shared MySQL without tenant scoping guard

Added a required OrganizationIDs []uint field to IDPOptions and a new inOrganization helper (matching the finding's referenced convention) in tools/dibble/pkg/seed/idp.go. IDP() now rejects the call up front if OrganizationIDs is empty, and filters both fetchUsersForIDP and fetchHostsForIDP results through inOrganization before any mutation, so only in-scope users/hosts reach upsertIDPAccount, upsertSCIMUser, assignHostToIDPAccount, and assignHostToSCIMUser. I also added OrganizationID uint fields to the idpUser/idpHost structs (json:"organization_id") since filtering requires the API to expose that field. This is LOW confidence because: (a) I cannot verify the Fleet /users and /hosts list endpoints actually return an organization_id field in their JSON payload as named here — if the real field name/shape differs, filtering will silently drop everything (empty users/hosts after filtering, producing the "no users found in scope" error) rather than compile-failing; (b) I have not seen the actual inOrganization helper implementation referenced by the finding elsewhere in the repo, so this is a new, possibly divergent definition rather than a shared import — a complete fix would locate and reuse the existing helper/pattern instead of redefining it locally; (c) callers of IDP() elsewhere in the dibble CLI (not shown) must now be updated to pass OrganizationIDs, which I could not do since that file is out of scope here — until that wiring lands, this seeder will always error out.

🤖 Prompt for AI agents
In tools/dibble/pkg/seed/idp.go around line 59, review and complete this code-review fix: dibble IDP seeder writes directly to shared MySQL without tenant scoping guard.
What the draft fix changed: Added a required `OrganizationIDs []uint` field to `IDPOptions` and a new `inOrganization` helper (matching the finding's referenced convention) in `tools/dibble/pkg/seed/idp.go`. `IDP()` now rejects the call up front if `OrganizationIDs` is empty, and filters both `fetchUsersForIDP` and `fetchHostsForIDP` results through `inOrganization` before any mutation, so only in-scope users/hosts reach `upsertIDPAccount`, `upsertSCIMUser`, `assignHostToIDPAccount`, and `assignHostToSCIMUser`. I also added `OrganizationID uint` fields to the `idpUser`/`idpHost` structs (`json:"organization_id"`) since filtering requires the API to expose that field. This is LOW confidence because: (a) I cannot verify the Fleet `/users` and `/hosts` list endpoints actually return an `organization_id` field in their JSON payload as named here — if the real field name/shape differs, filtering will silently drop everything (empty `users`/`hosts` after filtering, producing the "no users found in scope" error) rather than compile-failing; (b) I have not seen the actual `inOrganization` helper implementation referenced by the finding elsewhere in the repo, so this is a new, possibly divergent definition rather than a shared import — a complete fix would locate and reuse the existing helper/pattern instead of redefining it locally; (c) callers of `IDP()` elsewhere in the `dibble` CLI (not shown) must now be updated to pass `OrganizationIDs`, which I could not do since that file is out of scope here — until that wiring lands, this seeder will always error out.
The fix is LOW CONFIDENCE — verify it is correct and finish whatever it left incomplete.

fix confidence: 🔴 30 low — review closely — react 👍/👎 to teach the reviewer

@flamingo flamingo Bot changed the title fix(OPENFRAM-005-11): dibble IDP seeder writes directly to shared MySQL without tenant scoping guard fix(OPENFRAM-005-11): CU-86akj32d8 dibble IDP seeder writes directly to shared MySQL without tenant scoping guard Sep 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants