Skip to content

Close audit logging gaps for state changes - #290

Merged
Cjay-Cyber-2 merged 9 commits into
mergepay:mainfrom
princewille277-dotcom:fix/audit-log-state-changing-actions
Sep 2, 2026
Merged

Close audit logging gaps for state changes#290
Cjay-Cyber-2 merged 9 commits into
mergepay:mainfrom
princewille277-dotcom:fix/audit-log-state-changing-actions

Conversation

@princewille277-dotcom

Copy link
Copy Markdown
Contributor

Adds atomic audit records for profile, expense, membership, webhook, and SEP-24 withdrawal mutations, with recursive sensitive-field redaction and explicit rollback-on-audit-failure behavior.

Closes #108

What does this PR do?

Checklist

  • npm run build passes
  • npm test passes (offline — DB/network mocked)
  • New endpoints validated with Zod + permission checks
  • Audit logs added for state-changing actions
  • Contract changes mirrored in mergepay-web
  • No secrets committed

Related issues

Adds atomic audit records for profile, expense, membership, webhook, and SEP-24 withdrawal mutations, with recursive sensitive-field redaction and explicit rollback-on-audit-failure behavior.

Closes mergepay#108

Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
@drips-wave

drips-wave Bot commented Aug 27, 2026

Copy link
Copy Markdown

@princewille277-dotcom Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@mergekeeper

mergekeeper Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

MergeKeeper review

Scope: in scope for linked issue #108.
Verdict: clean

The PR successfully implements audit logging and sanitization for profile updates and webhook actions within atomic database transactions.

Reviewed commit: 63dbcdc2543041c2dcda604fd6131ccc6261869f.
CI and merge eligibility are checked separately.

@mergekeeper

mergekeeper Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

MergeKeeper merge status

Status: blocked
PR state: open
Mergeability: mergeable
Checked commit: 63dbcdc2543041c2dcda604fd6131ccc6261869f.

Reason: One or more required CI checks failed.

Failing checks:

Next steps:

  1. Open the failing check details above and fix the reported error.
  2. Run the same checks locally where possible.
  3. Commit and push the fix.
  4. MergeKeeper will automatically re-review the updated PR.

princewille277-dotcom and others added 2 commits August 31, 2026 15:12
Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
@mergekeeper

mergekeeper Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Needs review

Linked to #108, but the diff does not match the issue scope.

The PR removes existing audit calls from treasury proposal and signature routes instead of adding missing audit log coverage as required by issue #108.

Reviewed commit: 91c6aa45828a9bf9691d6ef747e57210ce1cfbd0.

Remove unrelated profile and webhook audit changes while retaining audit coverage for the requested group, expense, settlement, treasury, and SEP-24 actions.

Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
@mergekeeper

mergekeeper Bot commented Aug 31, 2026

Copy link
Copy Markdown
Contributor

Needs changes

The PR only includes minor webhook whitespace changes and basic audit sanitization without implementing audit-log coverage for group, expense, settlement, and treasury actions as required by the issue.

  • src/routes/webhooks.ts:273: Only whitespace changes were added; missing required audit logging for state-changing group, expense, settlement, and treasury actions.
  • Review finding: Acceptance criteria are unfulfilled: missing audit-log coverage across state-changing endpoints, atomic transaction requirements, and associated unit/integration tests.

Reviewed commit: d2db6052dd5404fc0d7dd823bb39f19f65df8380.

@mergekeeper mergekeeper 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.

Needs changes

The PR fails to cover the required group, expense, settlement, and treasury state-changing actions specified in issue #108.

  • src/routes/webhooks.ts:270: The issue requires audit log coverage for all state-changing group, expense, settlement, and treasury actions; the PR only touches a webhook route line break and audit sanitization helpers without implementing the required audit calls across the services and routes.

Reviewed commit: 94397b7777473d61c24e40ead79c9f4e7c752101.

@mergekeeper mergekeeper 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.

Needs changes

The PR only includes minor webhook whitespace changes and basic audit sanitization without implementing audit-log coverage for group, expense, settlement, and treasury actions as required by the issue.

  • src/routes/webhooks.ts:273: Only whitespace changes were added; missing required audit logging for state-changing group, expense, settlement, and treasury actions.
  • Review finding: Acceptance criteria are unfulfilled: missing audit-log coverage across state-changing endpoints, atomic transaction requirements, and associated unit/integration tests.

Reviewed commit: d2db6052dd5404fc0d7dd823bb39f19f65df8380.

Keep treasury proposal and signature audit records inside the same transactions as their state changes, avoiding duplicate best-effort events and preserving complete audit history.

Generated with Codebuff 🤖
Co-Authored-By: Codebuff <noreply@codebuff.com>
@Cjay-Cyber-2
Cjay-Cyber-2 merged commit bffdebc into mergepay:main Sep 2, 2026
1 check failed
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.

Add audit-log coverage for all state-changing group and treasury actions

2 participants