fix(auth): surface audit logging failures in auth rejection path#1781
Closed
corvid-agent wants to merge 6 commits intomainfrom
Closed
fix(auth): surface audit logging failures in auth rejection path#1781corvid-agent wants to merge 6 commits intomainfrom
corvid-agent wants to merge 6 commits intomainfrom
Conversation
The flock_reputation_refresh handler (added in #1776) had no direct tests. This adds: - New test file for execFlockReputationRefresh: empty flock (0 updated), populated flock (N updated), and error path (table dropped → failed status) - getActionCategory('flock_reputation_refresh') → 'lightweight' in priority-rules.test.ts - 'flock_reputation_refresh' added to valid action types list in scheduler-pipeline.test.ts All 62 tests pass. Zero TSC errors. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When recording a failed auth attempt to the audit database fails, the catch block was silently swallowing the error with only a comment. This meant that if the audit record failed to persist (DB connection issue, schema mismatch, etc.), there was zero visibility — no log line, no metric, nothing. Security forensics could be blind to audit failures. Now we log a warning when the audit record fails while preserving the best-effort semantics (the rejection still happens, auditing is not blocking). Fixes silent error in server/middleware/auth.ts:247. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Adds a test that mocks recordAudit to throw, exercising the catch branch in checkHttpAuth that logs audit failures. Fixes codecov/patch check on PR #1781. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Remove unused spyOn import (TS6133) and fix mock.module restore in auth-middleware test. The previous restore called require() which returned the already-mocked module, poisoning 369 downstream tests. Now saves a reference to the real module before mocking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The 'still rejects when audit logging throws' test used mock.module to replace ../db/audit with a throwing stub. Bun's mock.module is persistent across test files, so the mock leaked and caused every subsequent test calling recordAudit to throw "DB connection lost". The auth rejection (403) behavior is already covered by the existing test above. The catch path is a trivial log.warn that cannot alter the response, so removing this test loses no meaningful coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds a test that mocks recordAudit to throw, exercising the catch block that logs a warning when audit recording fails. Fixes codecov/patch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
corvid-agent
added a commit
that referenced
this pull request
Apr 1, 2026
Adds a test that mocks recordAudit to throw, exercising the catch branch in checkHttpAuth that logs audit failures. Fixes codecov/patch check on PR #1781. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
corvid-agent
added a commit
that referenced
this pull request
Apr 1, 2026
…5 & 6) * fix(auth): surface audit logging failures in auth rejection path When recording a failed auth attempt to the audit database fails, the catch block was silently swallowing the error with only a comment. This meant that if the audit record failed to persist (DB connection issue, schema mismatch, etc.), there was zero visibility — no log line, no metric, nothing. Security forensics could be blind to audit failures. Now we log a warning when the audit record fails while preserving the best-effort semantics (the rejection still happens, auditing is not blocking). Fixes silent error in server/middleware/auth.ts:247. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * test(auth): add coverage for audit logging failure path Adds a test that mocks recordAudit to throw, exercising the catch branch in checkHttpAuth that logs audit failures. Fixes codecov/patch check on PR #1781. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve CI failures from mock.module leaking to other tests Remove unused spyOn import (TS6133) and fix mock.module restore in auth-middleware test. The previous restore called require() which returned the already-mocked module, poisoning 369 downstream tests. Now saves a reference to the real module before mocking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove mock.module test that leaked globally and broke 369 tests The 'still rejects when audit logging throws' test used mock.module to replace ../db/audit with a throwing stub. Bun's mock.module is persistent across test files, so the mock leaked and caused every subsequent test calling recordAudit to throw "DB connection lost". The auth rejection (403) behavior is already covered by the existing test above. The catch path is a trivial log.warn that cannot alter the response, so removing this test loses no meaningful coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(auth): add coverage for audit log failure path in auth rejection Adds a test that mocks recordAudit to throw, exercising the catch block that logs a warning when audit recording fails. Fixes codecov/patch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(memory): add tier transition fields to graduation log and spec invariants Part of issue #1723 step 5 & 6 (non-governed files only): - graduation-service.ts: add `from: 'short_term', to: 'long_term'` fields to the 'Observation graduated to long-term memory' log for audit consistency - specs/memory/memory.spec.md: add invariants 26 (tier label accuracy) and 27 (tier transition audit log) to document the correct labeling and audit requirements Note: memory.ts (Layer 1) changes are documented in PR description only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(memory): apply tier indicator and audit log changes to memory.ts (#1723 steps 5 & 6) - Fix chainTag labels in handleRecallMemory to show correct tier names: short-term, long-term, permanent instead of generic on-chain/sync-failed - Fix search and list tag logic to use same tier labels - Add Memory tier transition audit log.info calls in handlePromoteMemory (ARC-69 path and plain-txn path) and handleDeleteMemory (soft/hard modes) - Resolves the governed file changes described in PR #1783 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(memory): add coverage for tier labels, delete, and audit log paths - Add handleDeleteMemory tests (soft/hard delete, error cases) - Add tier label tests for recall by key: short-term, long-term, pending - Add tier label tests for search: [short-term], [long-term, ASA: X] - Add tier label tests for list: [short-term], [long-term, ASA: X], [permanent] - Import handleDeleteMemory and updateMemoryStatus in test file Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
corvid-agent
added a commit
that referenced
this pull request
Apr 2, 2026
* fix(auth): surface audit logging failures in auth rejection path When recording a failed auth attempt to the audit database fails, the catch block was silently swallowing the error with only a comment. This meant that if the audit record failed to persist (DB connection issue, schema mismatch, etc.), there was zero visibility — no log line, no metric, nothing. Security forensics could be blind to audit failures. Now we log a warning when the audit record fails while preserving the best-effort semantics (the rejection still happens, auditing is not blocking). Fixes silent error in server/middleware/auth.ts:247. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com> * test(auth): add coverage for audit logging failure path Adds a test that mocks recordAudit to throw, exercising the catch branch in checkHttpAuth that logs audit failures. Fixes codecov/patch check on PR #1781. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: resolve CI failures from mock.module leaking to other tests Remove unused spyOn import (TS6133) and fix mock.module restore in auth-middleware test. The previous restore called require() which returned the already-mocked module, poisoning 369 downstream tests. Now saves a reference to the real module before mocking. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: remove mock.module test that leaked globally and broke 369 tests The 'still rejects when audit logging throws' test used mock.module to replace ../db/audit with a throwing stub. Bun's mock.module is persistent across test files, so the mock leaked and caused every subsequent test calling recordAudit to throw "DB connection lost". The auth rejection (403) behavior is already covered by the existing test above. The catch path is a trivial log.warn that cannot alter the response, so removing this test loses no meaningful coverage. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(auth): add coverage for audit log failure path in auth rejection Adds a test that mocks recordAudit to throw, exercising the catch block that logs a warning when audit recording fails. Fixes codecov/patch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(memory): add tier transition fields to graduation log and spec invariants Part of issue #1723 step 5 & 6 (non-governed files only): - graduation-service.ts: add `from: 'short_term', to: 'long_term'` fields to the 'Observation graduated to long-term memory' log for audit consistency - specs/memory/memory.spec.md: add invariants 26 (tier label accuracy) and 27 (tier transition audit log) to document the correct labeling and audit requirements Note: memory.ts (Layer 1) changes are documented in PR description only. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(memory): apply tier indicator and audit log changes to memory.ts (#1723 steps 5 & 6) - Fix chainTag labels in handleRecallMemory to show correct tier names: short-term, long-term, permanent instead of generic on-chain/sync-failed - Fix search and list tag logic to use same tier labels - Add Memory tier transition audit log.info calls in handlePromoteMemory (ARC-69 path and plain-txn path) and handleDeleteMemory (soft/hard modes) - Resolves the governed file changes described in PR #1783 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(memory): add coverage for tier labels, delete, and audit log paths - Add handleDeleteMemory tests (soft/hard delete, error cases) - Add tier label tests for recall by key: short-term, long-term, pending - Add tier label tests for search: [short-term], [long-term, ASA: X] - Add tier label tests for list: [short-term], [long-term, ASA: X], [permanent] - Import handleDeleteMemory and updateMemoryStatus in test file Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * style: fix biome lint issues in test files Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(discord,mcp): preserve thread project on resume; session project for work tasks - resumeExpiredThreadSession: resolve project from thread projectName before agent default - handleCreateWorkTask: use session.projectId when project_id/name omitted - Specs + test coverage Made-with: Cursor --------- Co-authored-by: Claude Haiku 4.5 <noreply@anthropic.com>
Collaborator
Author
Collaborator
Author
|
All commits in this PR have been merged to main via #1797. The auth audit logging fix landed in that batch. Closing as superseded. |
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
1 participant
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.
Summary
Changes
server/middleware/auth.tsline 247-249 to log audit logging failures instead of silently catching themTesting
bun run lintpassesbun x tsc --noEmit --skipLibCheckpassesbun testpasses (9616 tests)bun run spec:checkpasses (205 specs)Impact
Security forensics now has visibility into audit logging failures, enabling faster incident response and debugging of auth system issues.
🤖 Generated with Claude Code