Skip to content

Secfix/sso takeover#19173

Merged
calexiou merged 10 commits into
masterfrom
secfix/sso-takeover
Jul 9, 2026
Merged

Secfix/sso takeover#19173
calexiou merged 10 commits into
masterfrom
secfix/sso-takeover

Conversation

@calexiou

@calexiou calexiou commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Mitigates -> GHSA-hp6v-6jw7-gv2f

Review in cubic

@calexiou calexiou self-assigned this Jul 8, 2026
@readme-ai-writer

readme-ai-writer Bot commented Jul 8, 2026

Copy link
Copy Markdown

📚 No documentation changes were needed for this PR.

@github-actions github-actions Bot added size/m and removed size/xs labels Jul 8, 2026

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found and verified against the latest diff

Confidence score: 2/5

  • In packages/types/src/documents/global/config.ts, allowUnverifiedEmailLinking is defined in the config types but appears not to be mapped through fetchStrategyConfig, so the runtime strategy may ignore this flag and allow unintended OIDC account-linking behavior if merged as-is. Wire the field through fetchStrategyConfig and add a focused test that verifies persisted config and runtime config stay in sync before merging.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/types/src/documents/global/config.ts">

<violation number="1" location="packages/types/src/documents/global/config.ts:88">
P0: The `allowUnverifiedEmailLinking` field is added to both `OIDCInnerConfig` and `OIDCStrategyConfiguration`, but `fetchStrategyConfig` (the function that maps from the stored config to the runtime strategy config) does not propagate it. The destructuring on the input and the return object both omit this field, so it will be silently dropped at runtime — the feature will have no effect.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/types/src/documents/global/config.ts

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 8 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/backend-core/src/middleware/passport/sso/google.ts Outdated
Co-authored-by: cubic-dev-ai[bot] <191113872+cubic-dev-ai[bot]@users.noreply.github.com>
Comment thread packages/builder/src/settings/pages/auth/index.svelte

@melohagan melohagan left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just noticed a possible null-pointer, otherwise looks ok

Comment thread packages/backend-core/src/middleware/passport/sso/oidc.ts
Comment thread packages/builder/src/settings/pages/auth/index.svelte
@calexiou
calexiou merged commit 007ca6b into master Jul 9, 2026
34 checks passed
@calexiou
calexiou deleted the secfix/sso-takeover branch July 9, 2026 10:24
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 9, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants