feat: allow disabling verification hook auto-inject

[David-Wobrock]

Related issue(s)

Fixes #4437

Checklist

• I have read the contributing guidelines.
• I have referenced an issue containing the design document if my change
  introduces a new feature.
• I am following the
  contributing code guidelines.
• I have read the security policy.
• I confirm that this pull request does not address a security
  vulnerability. If this pull request addresses a security vulnerability, I
  confirm that I got the approval (please contact
  security@ory.com) from the maintainers to push
  the changes.
• I have added tests that prove my fix is effective or that my feature
  works.
• I have added or changed the documentation.

Further Comments

Summary by CodeRabbit

• New Features
  • Added feature_flags.disable_verification_hook_auto_injection to control whether verification hooks are automatically inserted in self-service registration and settings flows.
  • Added support for the selfServiceVerificationHook hook type in the self-service hook schema.
• Behavior Changes / Bug Fixes
  • Verification-hook auto-injection is now gated by having self-service verification enabled and auto-injection not disabled; explicit verification hooks still take effect.
• Tests
  • Updated and expanded coverage for default vs explicit hook behavior, and to confirm no verification emails are sent when auto-injection is disabled.

[coderabbitai]

📝 Walkthrough

Walkthrough

A new boolean feature flag feature_flags.disable_verification_hook_auto_injection (default false) is introduced. When enabled, the automatic injection of the verifier hook into registration and settings post-persist hook chains is suppressed, even when self-service verification is globally enabled. The flag is exposed via a config constant, a *Config accessor method, updated JSON schema definitions, guarded conditions in the registration and settings registries, and comprehensive tests.

Changes

Verification Hook Auto-Injection Toggle

Layer / File(s)	Summary
Feature flag config key, method, and schema driver/config/config.go, embedx/config.schema.json	Adds ViperKeyDisableVerificationHookAutoInjection constant, SelfServiceVerificationHookAutoInjectionDisabled(ctx) method, a new feature_flags.disable_verification_hook_auto_injection boolean in the JSON schema, and selfServiceVerificationHook as an explicit anyOf option in the registration and settings profile hook lists.
Registry hook injection guard driver/registry_default_registration.go, driver/registry_default_settings.go	PostRegistrationPostPersistHooks and PostSettingsPostPersistHooks now require both SelfServiceFlowVerificationEnabled and !SelfServiceVerificationHookAutoInjectionDisabled before inserting the verifier hook.
Tests driver/config/config_test.go, driver/registry_default_test.go, selfservice/hook/verification_test.go	Config test asserts default false and toggle to true. Registry test adds registration and settings matrix cases for the disabled auto-injection scenario. Hook test adds a TestVerifier subtest confirming no auto-injected verifier hook and no courier messages when the flag is active.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check	❓ Inconclusive	The pull request description includes a reference to the linked issue (#4437) but lacks detailed implementation description, technical justification, or explanation of how the feature works.	Add a detailed description of the implementation approach, explain why the new feature flag was chosen, and describe how users can use this feature.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat: allow disabling verification hook auto-inject' directly matches the main objective and accurately summarizes the primary change introduced in the pull request.
Linked Issues check	✅ Passed	All coding requirements from issue #4437 are met: a configuration mechanism to disable verification hook auto-injection is implemented for both registration and settings flows, backward compatibility is maintained with default behavior unchanged.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the feature flag for disabling verification hook auto-injection as specified in issue #4437; no extraneous modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@selfservice/hook/verification_test.go`:
- Around line 153-180: The test case "should not send verification emails when
auto-injection is disabled" bypasses the auto-injection validation by manually
executing hook.NewVerifier(reg) on line 173, which does not test the intended
behavior. Remove the manual hook execution (lines 173-174: `h :=
hook.NewVerifier(reg)` and `tc.execHook(h, i, originalFlow)`) and instead
execute the hooks retrieved from reg.PostRegistrationPostPersistHooks on line
165, then assert that zero messages are queued (change the require.Len assertion
to expect 0 messages instead of 1). This directly validates that disabling
auto-injection prevents the verifier hook from being called in the normal hook
chain.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8405619d-0a4f-4496-9de3-77deaf28c512

📥 Commits

Reviewing files that changed from the base of the PR and between c445e40 and 7f885bd.

📒 Files selected for processing (7)

• driver/config/config.go
• driver/config/config_test.go
• driver/registry_default_registration.go
• driver/registry_default_settings.go
• driver/registry_default_test.go
• embedx/config.schema.json
• selfservice/hook/verification_test.go

[coderabbitai]

🧹 Nitpick comments (1)

selfservice/hook/verification_test.go (1)

153-175: ⚡ Quick win

Move this subtest out of the flow loop to avoid duplicate/misleading execution.

This case validates registration hook auto-injection only, but it runs under both flow=login and flow=registration and does not use tc. Consider moving it outside the for _, tc := ... loop (or parameterizing by hook family) so test output matches intent and avoids duplicate runs.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@selfservice/hook/verification_test.go` around lines 153 - 175, The subtest
"case=should not send verification emails when auto-injection is disabled" is
currently nested inside a loop that iterates over different flows
(login/registration), but this test only validates registration hook
auto-injection behavior and does not use the loop variable. Move this t.Run call
outside the for loop so it executes only once with the correct context,
preventing duplicate/misleading test runs under unintended flow configurations.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@selfservice/hook/verification_test.go`:
- Around line 153-175: The subtest "case=should not send verification emails
when auto-injection is disabled" is currently nested inside a loop that iterates
over different flows (login/registration), but this test only validates
registration hook auto-injection behavior and does not use the loop variable.
Move this t.Run call outside the for loop so it executes only once with the
correct context, preventing duplicate/misleading test runs under unintended flow
configurations.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c97dc315-db8f-4998-af7a-5c90a7fc62e4

📥 Commits

Reviewing files that changed from the base of the PR and between 7f885bd and 3b8cfcf.

📒 Files selected for processing (7)

• driver/config/config.go
• driver/config/config_test.go
• driver/registry_default_registration.go
• driver/registry_default_settings.go
• driver/registry_default_test.go
• embedx/config.schema.json
• selfservice/hook/verification_test.go

🚧 Files skipped from review as they are similar to previous changes (6)

• driver/registry_default_registration.go
• driver/config/config.go
• driver/registry_default_test.go
• driver/registry_default_settings.go
• driver/config/config_test.go
• embedx/config.schema.json

[coderabbitai]

🧹 Nitpick comments (1)

driver/config/config_test.go (1)

641-643: 💤 Low value

Consider moving the test to a more semantically appropriate function.

The TestSession function tests session-related configurations (session name, lifespan, persistent cookie, whoami caching). The verification hook auto-injection test at lines 641-643 is about post-registration/settings hooks, not sessions. While the test is functionally correct, placing it in a dedicated test function (e.g., TestVerificationHookAutoInjection or grouping it with other verification-related tests) would improve code organization and make it easier for future maintainers to locate.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@driver/config/config_test.go` around lines 641 - 643, The test code at lines
641-643 testing SelfServiceVerificationHookAutoInjectionDisabled is currently
located in the TestSession function, but it tests verification hook
auto-injection configuration rather than session-related configuration. Move
this test snippet (the three assert lines testing the verification hook
auto-injection disabled state) out of TestSession and into a new dedicated test
function named TestVerificationHookAutoInjection or similar. This improves code
organization by semantically grouping related tests together and makes it easier
for maintainers to locate verification hook-related tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@driver/config/config_test.go`:
- Around line 641-643: The test code at lines 641-643 testing
SelfServiceVerificationHookAutoInjectionDisabled is currently located in the
TestSession function, but it tests verification hook auto-injection
configuration rather than session-related configuration. Move this test snippet
(the three assert lines testing the verification hook auto-injection disabled
state) out of TestSession and into a new dedicated test function named
TestVerificationHookAutoInjection or similar. This improves code organization by
semantically grouping related tests together and makes it easier for maintainers
to locate verification hook-related tests.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 388ecae3-3b96-4c73-9df3-4a244a63056f

📥 Commits

Reviewing files that changed from the base of the PR and between 3b8cfcf and d0b2689.

📒 Files selected for processing (7)

• driver/config/config.go
• driver/config/config_test.go
• driver/registry_default_registration.go
• driver/registry_default_settings.go
• driver/registry_default_test.go
• embedx/config.schema.json
• selfservice/hook/verification_test.go

🚧 Files skipped from review as they are similar to previous changes (4)

• selfservice/hook/verification_test.go
• driver/config/config.go
• embedx/config.schema.json
• driver/registry_default_test.go

[coderabbitai]

🧹 Nitpick comments (1)

driver/config/config_test.go (1)

642-651: 💤 Low value

Test correctly verifies config-level flag behavior.

The test appropriately checks the default value (false, preserving backward compatibility) and the setter functionality. Well done.

♻️ Optional: use assert.True/False for slightly more idiomatic boolean checks

-	assert.Equal(t, false, p.SelfServiceVerificationHookAutoInjectionDisabled(ctx))
+	assert.False(t, p.SelfServiceVerificationHookAutoInjectionDisabled(ctx))
 	p.MustSet(ctx, config.ViperKeyDisableVerificationHookAutoInjection, true)
-	assert.Equal(t, true, p.SelfServiceVerificationHookAutoInjectionDisabled(ctx))
+	assert.True(t, p.SelfServiceVerificationHookAutoInjectionDisabled(ctx))

Note: Both forms are valid; this file uses both patterns (e.g., lines 62, 66 vs. line 234), so the current code is consistent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@driver/config/config_test.go` around lines 642 - 651, The test
TestVerificationHookAutoInjection is well-structured and correctly verifies the
config flag behavior with both default and set values. As an optional
improvement for more idiomatic Go testing, consider refactoring the two
assert.Equal calls that check boolean values (the assertions comparing to false
and true respectively) to use assert.False and assert.True instead. Note that
this is optional since the file already consistently uses both patterns
throughout, so maintaining the current assert.Equal style is also acceptable.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@driver/config/config_test.go`:
- Around line 642-651: The test TestVerificationHookAutoInjection is
well-structured and correctly verifies the config flag behavior with both
default and set values. As an optional improvement for more idiomatic Go
testing, consider refactoring the two assert.Equal calls that check boolean
values (the assertions comparing to false and true respectively) to use
assert.False and assert.True instead. Note that this is optional since the file
already consistently uses both patterns throughout, so maintaining the current
assert.Equal style is also acceptable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6ed7feb2-140d-42f3-abce-a533a5030259

📥 Commits

Reviewing files that changed from the base of the PR and between d0b2689 and 5baacab.

📒 Files selected for processing (7)

• driver/config/config.go
• driver/config/config_test.go
• driver/registry_default_registration.go
• driver/registry_default_settings.go
• driver/registry_default_test.go
• embedx/config.schema.json
• selfservice/hook/verification_test.go

🚧 Files skipped from review as they are similar to previous changes (6)

• driver/registry_default_settings.go
• embedx/config.schema.json
• driver/registry_default_registration.go
• selfservice/hook/verification_test.go
• driver/registry_default_test.go
• driver/config/config.go