feat: access-log filtering, revoke-access status update, dispute status audit, user rate limiting - #1227
Merged
mftee merged 2 commits intoAug 26, 2026
Conversation
…us audit, user rate limiting
Contributor
|
@portableDD is attempting to deploy a commit to the Mftee's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@portableDD 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! 🚀 |
Resolve conflicts: - access-logs.service.ts: kept CodeGirlsInc#1222's already-merged DI fix (@InjectRepository decorator was missing on this PR's version). - users.service.spec.ts (add/add): both versions test different, non-overlapping real methods on UsersService (create/findById/ findByEmail/update/softDelete vs. this PR's new checkRateLimit) - combined into one file. - dispute.controller.ts merged cleanly, but fixed a real authorization gap while here: the new PATCH /disputes/:id/status endpoint had no role check at all, so any authenticated user could resolve/reject any other user's dispute. Added the same admin-only check already used consistently elsewhere in this exact file (fileDispute, getDispute, and documents.controller.ts's revokeAccess all gate on ownership-or-admin) - the new endpoint was clearly missed.
mftee
approved these changes
Aug 26, 2026
mftee
left a comment
mftee
left a comment
Contributor
There was a problem hiding this comment.
Reviewed. Good work overall:
- New
PATCH /:id/revokeon documents correctly gates on ownership-or-admin, matching #1225's convention exactly. - Real, working
checkRateLimitadded to UsersService (in-memory sliding window, 30/min), verified against the actual implementation — though it's not wired into any endpoint yet, so it's inert for now. - Access-log filtering DTO and dispute status field/history look solid.
Found and fixed one real issue before merging: the new PATCH /disputes/:id/status endpoint had no authorization check at all — any authenticated user could resolve/reject/reopen any dispute, including ones filed by other users. This is a genuine broken-access-control gap (and inconsistent with this same PR's own revokeAccess endpoint, which does gate correctly), so I've added the same admin-only check already used consistently elsewhere in this file for dispute actions.
Also resolved: an access-logs.service.ts conflict against #1222's already-merged DI fix, and an add/add test-file conflict in users.service.spec.ts (combined, both compatible).
Approving.
4 tasks
mftee
added a commit
to devzakari-sketch/SMALDA
that referenced
this pull request
Aug 26, 2026
Resolve conflicts and fix genuine corruption in this PR's own commit:
1. Multiple template literals in this PR lost their backticks and
${...} interpolation entirely, producing invalid syntax (verified
against the raw source on the PR branch, not just the diff):
- dispute.service.ts: 'throw new NotFoundException(Dispute not
found);' (x2) and a similarly broken BadRequestException message.
- documents.controller.ts: 'const filename = ${storageKey};' (lost
the safeExtension suffix too), a broken quota-exceeded message,
and 'Content-Disposition': ttachment; filename="",' (also
containing a stray literal bell character where a backtick should
have been). Restored all five to their clearly-intended form -
three are exact matches of the original code this PR overwrote.
2. dispute.service.ts / dispute.entity.ts: merged this PR's new
ALLOWED_DISPUTE_TRANSITIONS state-machine validation with CodeGirlsInc#1227's
already-merged audit-logging updateStatus() - both needed together.
Also discovered this PR's DisputeStatus enum (OPEN/UNDER_REVIEW/
RESOLVED) doesn't match origin/main's already-merged enum (OPEN/
IN_REVIEW/RESOLVED/DISMISSED, backed by a proper Postgres enum
column) - kept the already-merged, stricter version and adapted
the new transition map to it.
3. Fixed two silent duplicate-key merge artifacts git produced without
flagging a conflict: 'status: DisputeStatus' appeared twice in both
dispute.service.ts's toResponseDto() and dispute-response.dto.ts.
4. access-logs.service.ts, users.service.spec.ts: standard conflicts
against CodeGirlsInc#1222/CodeGirlsInc#1227, resolved consistently with prior merges.
5. logger.config.ts: this PR's test asserts that a camelCase
'stellarSecretKey' field gets redacted, but the real
isSensitiveKey() only matched snake_case keys - a live Stellar
secret key logged via a camelCase object property (the normal JS/TS
convention) would not have been redacted. Fixed by normalizing
camelCase to snake_case before the sensitivity check.
mftee
added a commit
that referenced
this pull request
Aug 26, 2026
…-security-and-validation Merging per repo maintainer review. Fixed genuine syntax-breaking corruption in this PR's own commit (5 broken template literals across 2 files, verified against raw source) before merging, plus resolved conflicts against #1227 (dispute status transitions/enum) and #1222 (access-logs DI). Also fixed a real camelCase-secret-redaction gap this PR's own test surfaced. Pre-existing CI failures otherwise predate this PR.
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
2 participants
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.
Closes #1149
Closes #1148
Closes #1147
Closes #1146