WPB-20208 Audit log on asset download #4784
Merged
Conversation
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
battermann
force-pushed
the
WPB-20208-add-audit-logging-to-asset-download
branch
from
72cc58d to
f1b0322
Compare
zebot
added
the
ok-to-test
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements comprehensive audit logging for asset operations in the Cargohold service, extending beyond upload events to include download, signed URL creation, and remote asset download logging.
Key changes include:
- Enhanced audit logging functions to cover download, signed URL creation, and remote asset operations
- Updated asset handling functions to include audit metadata and qualified principals for proper logging
- Comprehensive unit tests with property-based testing to validate audit log JSON structure
- Integration tests covering various federated download scenarios across multiple backends
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| services/cargohold/src/CargoHold/API/AuditLog.hs | Expanded audit logging with new functions for download, URL creation, and remote access |
| services/cargohold/src/CargoHold/API/V3.hs | Updated asset operations to support qualified principals and audit metadata |
| services/cargohold/src/CargoHold/Util.hs | Modified signed URL generation to include audit logging |
| services/cargohold/src/CargoHold/S3.hs | Added Arbitrary instances and cleaned up type signatures |
| services/cargohold/test/unit/Test/CargoHold/API/AuditLogTest.hs | Property-based tests for audit log JSON validation |
| services/cargohold/test/unit/Test/CargoHold/API/LogJSON.hs | Test utilities for structured JSON logging |
| integration/test/Test/Cargohold/API.hs | Enhanced integration tests for federated audit logging scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
services/cargohold/test/unit/Test/CargoHold/API/AuditLogTest.hs
Outdated
Show resolved
Hide resolved
services/cargohold/test/unit/Test/CargoHold/API/AuditLogTest.hs
Outdated
Show resolved
Hide resolved
battermann
force-pushed
the
WPB-20208-add-audit-logging-to-asset-download
branch
from
c825167 to
890a52a
Compare
blackheaven
approved these changes
Sep 26, 2025
Comment on lines
+20
to
+21
| lookupText, | ||
| lookupBool, |
Contributor
There was a problem hiding this comment.
they don't seems to be used, maybe we can rely on lens-aeson instead.
Contributor
Author
There was a problem hiding this comment.
Thanks, I'll remove these in the follow-up PR to reduce the number of rebases...
battermann
deleted the
WPB-20208-add-audit-logging-to-asset-download
branch
battermann
added a commit
that referenced
this pull request
Oct 2, 2025
This was referenced Oct 20, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
https://wearezeta.atlassian.net/browse/WPB-20208
Checklist
changelog.d