Extracted a shared action logger for the audit log#28899
Conversation
Gift-links and reset-authentication each wrote actions table rows directly, duplicating the best-effort write and actor mapping. Pulled both into a shared actionLogger and a fromFrame boundary helper so services record audit entries through one injectable seam instead of touching the model, and reset-auth now takes the request context as a parameter like gift-links does.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run ghost:test:ci:integration |
✅ Succeeded | 2m 36s | View ↗ |
nx run ghost:test:integration |
✅ Succeeded | 2m 37s | View ↗ |
nx run ghost:test:legacy |
✅ Succeeded | 2m 44s | View ↗ |
nx run ghost:test:e2e |
✅ Succeeded | 2m 17s | View ↗ |
nx run-many --target=build --projects=tag:publi... |
✅ Succeeded | 1s | View ↗ |
nx run-many -t lint -p ghost |
✅ Succeeded | 35s | View ↗ |
nx run-many -t test:unit -p ghost |
✅ Succeeded | 32s | View ↗ |
nx run @tryghost/admin:build |
✅ Succeeded | 7s | View ↗ |
Additional runs (2) |
✅ Succeeded | ... | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-06-25 13:25:22 UTC
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 `@ghost/core/core/server/services/auth/reset-authentication.ts`:
- Around line 32-35: The default logger is still bound to the global Action
model instead of the injected models dependency, which breaks the injectable
seam in resetAuthentication. Update the reset-authentication.ts setup so the
default logAction is created from the models parameter rather than
modelsDefault, using the existing actionLogger helper and the models.Action
symbol to keep audit writes aligned with whichever model store the caller
injects.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c4ca8d9b-34af-4883-9fc0-710222b859de
📒 Files selected for processing (10)
ghost/core/core/server/api/endpoints/authentication.jsghost/core/core/server/api/endpoints/gift-links.tsghost/core/core/server/api/endpoints/utils/request-context.tsghost/core/core/server/services/actions/index.tsghost/core/core/server/services/auth/reset-authentication.tsghost/core/core/server/services/gift-links/index.tsghost/core/core/server/services/gift-links/service.tsghost/core/test/integration/services/actions.test.tsghost/core/test/integration/services/gift-links.test.tsghost/core/test/unit/server/services/auth/reset-authentication.test.ts
| models = modelsDefault, | ||
| internalKeys = internalKeysDefault, | ||
| deleteAllSessions = deleteAllSessionsDefault | ||
| deleteAllSessions = deleteAllSessionsDefault, | ||
| logAction = actionLogger(modelsDefault.Action) |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Bind the default logger to the injected models dependency.
Line 35 currently uses modelsDefault.Action, so any caller that overrides models but relies on the default logAction still writes audit rows through the global Action model. That breaks the injectable seam this refactor is introducing and can send writes to the wrong model/store in tests or alternate integrations.
Suggested fix
- logAction = actionLogger(modelsDefault.Action)
+ logAction = actionLogger(models.Action)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| models = modelsDefault, | |
| internalKeys = internalKeysDefault, | |
| deleteAllSessions = deleteAllSessionsDefault | |
| deleteAllSessions = deleteAllSessionsDefault, | |
| logAction = actionLogger(modelsDefault.Action) | |
| models = modelsDefault, | |
| internalKeys = internalKeysDefault, | |
| deleteAllSessions = deleteAllSessionsDefault, | |
| logAction = actionLogger(models.Action) |
🤖 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 `@ghost/core/core/server/services/auth/reset-authentication.ts` around lines 32
- 35, The default logger is still bound to the global Action model instead of
the injected models dependency, which breaks the injectable seam in
resetAuthentication. Update the reset-authentication.ts setup so the default
logAction is created from the models parameter rather than modelsDefault, using
the existing actionLogger helper and the models.Action symbol to keep audit
writes aligned with whichever model store the caller injects.
1 participant
Problem
The gift-link audit-log change deliberately kept its action-recording logic local to the gift-links service rather than generalising from a single use case. There is now a second flow that records an audit action in the same way — the "reset all authentication" security action — so the logic for turning a domain action into an audit record, writing it best-effort, and identifying who acted is duplicated across two unrelated areas.
Solution
Extract a single shared action logger that both the gift-links and reset-authentication flows use. It owns the audit-record shape, the best-effort write to the actions table, and the request context that identifies the actor. The reset flow now records its audit after the fact through the same logger and takes the same request context as gift-links, so the two read consistently and the duplication is removed. With a second real consumer, the shared abstraction is justified rather than speculative.