Part 2 of IP events logic

mrheinen commented 2 months ago

PR Type

Enhancement, Bug fix

Description

Enhanced IP event management system with scan detection and aggregation
Added new fields to IP events: Subtype, Source, SourceRef, HoneypotIP
Updated backend, database, and UI to support new IP event features
Improved configuration options for scan detection and aggregation
Added constants for IP event sources and types
Enhanced string map cache functionality
Updated VirusTotal manager to use new IP event fields
Fixed query handling bugs in UI components
Added and updated tests for new functionality

Changes walkthrough 📝

Relevant files

Configuration changes

2 files

backend_main.go `Updated IP event manager initialization` cmd/backend/backend_main.go Updated `NewIpEventManagerImpl` function call with additional parameters	+1/-1
config.go `Added scan detection configuration` pkg/backend/config.go - Added new configuration options for scan detection and aggregation	+13/-2

Documentation

1 files

main.go

Added IP event documentation generation

cmd/gendoc/main.go - Added `WriteModelToFile` call for `database.IpEvent`

+1/-0

Enhancement

9 files

ip.go `Enhanced IP event management and scan detection` pkg/analysis/ip.go Moved IP event type constants to constants package Added scan detection and aggregation logic Updated `IpEventManagerImpl` struct with new fields Modified `ProcessNewEvent` function	+90/-28
server.go `Added IP event documentation handling` pkg/api/server.go - Added handling for 'ipevent' in `HandleReturnDocField` function	+2/-0
backend.go `Enhanced IP event creation in backend` pkg/backend/backend.go Updated IP event creation with new fields (Source, SourceRef, HoneypotIP) Modified event type constants to use the new constants package	+21/-12
database.go `Enhanced IP event database model` pkg/database/database.go Updated `IpEvent` struct with new fields (Subtype, Source, SourceRef, HoneypotIP)	+14/-10
shared_constants.go `Added IP event constants` pkg/util/constants/shared_constants.go - Added constants for IP event sources and types	+22/-0
string_map_cache.go `Enhanced string map cache functionality` pkg/util/string_map_cache.go Added `Update` and `GetDurationStored` methods to `StringMapCache` Modified `CacheEntry` struct to include `CreationTime`	+35/-5
manager.go `Updated VirusTotal manager for IP events` pkg/vt/manager.go Updated `GetEventsForDownload` to use new IP event fields and constants	+23/-11
EventsForm.vue `Added IP event subtype display` ui/src/components/container/EventsForm.vue - Added display for IP event subtype	+5/-0
EventsList.vue `Enhanced IP events list display` ui/src/components/container/EventsList.vue Updated search bar to use 'ipevent' model Added columns for Source and Source Ref Fixed query handling in mounted hook	+8/-2

Tests

2 files

ip_test.go `Updated and added tests for IP event manager` pkg/analysis/ip_test.go Updated test cases for new IP event manager functionality Added tests for scan event creation and duplication prevention	+118/-3
backend_test.go `Updated backend tests for IP events` pkg/backend/backend_test.go - Updated test cases to use new constants for IP event types	+8/-4

Bug fix

1 files

DownloadsList.vue

Fixed download list query handling

ui/src/components/container/DownloadsList.vue - Fixed query handling in mounted hook

+1/-0

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

github-actions[bot] commented 2 months ago

Preparing PR description...

github-actions[bot] commented 2 months ago

Preparing review...

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review

Possible Bug
The `CreateScanEvents` function uses a hardcoded scan threshold of 3, which might not be flexible enough for different scenarios. Performance Issue
The `CreateScanEvents` function iterates over all entries in the IP cache, which could be inefficient for large caches. Code Smell
The `ProcessRequest` function is using string concatenation for building the `Details` field, which could be inefficient for large strings.

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	✅ Add error handling for the scan cache update operation ___ *Consider adding error handling for the `i.scanCache.Update(ip, existingEvt)` call. If an error occurs during the update, it should be logged or handled appropriately.** [pkg/analysis/ip.go [152-154]](https://github.com/mrheinen/lophiid/pull/26/files#diff-24480f2fb374402d21f92840858fefced2bab4fd547efd664c76acbce047103bR152-R154) ```diff if duration < i.aggregateScanWindow { - i.scanCache.Update(ip, existingEvt) + if err := i.scanCache.Update(ip, existingEvt); err != nil { + slog.Error("Failed to update scan cache", slog.String("ip", ip), slog.String("error", err.Error())) + } } ``` `[Suggestion has been applied]` Suggestion importance[1-10]: 8 Why: Adding error handling for the cache update operation is important for robustness and debugging.	8
Error handling	Use a custom error type for more specific error handling ___ Consider using a more specific error type or custom error for the `fmt.Errorf("failed` `to replace cache entry: %w", err)` call. This would allow for more granular error handling in the calling code. [pkg/analysis/ip.go [182-184]](https://github.com/mrheinen/lophiid/pull/26/files#diff-24480f2fb374402d21f92840858fefced2bab4fd547efd664c76acbce047103bR182-R184) ```diff if err := i.ipCache.Replace(cacheKey, entry); err != nil { - return fmt.Errorf("failed to replace cache entry: %w", err) + return &CacheReplaceError{Err: err, Key: cacheKey} } ``` - [ ] Apply this suggestion* Suggestion importance[1-10]: 7 Why: Using a custom error type allows for more granular error handling, which can improve error management.	7
Best practice	Use a named constant for the scan threshold value ___ Consider using a constant or enum for the 'scanThreshold' value instead of a magic number. This would improve code readability and maintainability. [pkg/analysis/ip.go [117]](https://github.com/mrheinen/lophiid/pull/26/files#diff-24480f2fb374402d21f92840858fefced2bab4fd547efd664c76acbce047103bR117-R117) ```diff -const scanThreshold = 3 +const ScanThreshold = 3 ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 6 Why: Using a named constant improves code readability and maintainability, but it's a minor improvement.	6
Maintainability	Use a more descriptive variable name for better readability ___ Consider using a more descriptive variable name for 'cnt' to improve code readability. For example, 'eventCount' or 'scanEventCount' would be more informative. [pkg/analysis/ip.go [138]](https://github.com/mrheinen/lophiid/pull/26/files#diff-24480f2fb374402d21f92840858fefced2bab4fd547efd664c76acbce047103bR138-R138) ```diff -if cnt >= scanThreshold { +if eventCount >= scanThreshold { ``` - [ ] Apply this suggestion Suggestion importance[1-10]: 5 Why: A more descriptive variable name enhances code readability, but it's a relatively minor change.	5

codiumai-pr-agent-pro[bot] commented 2 months ago

CI Failure Feedback 🧐

**Action:** build

**Failed stage:** [Build](https://github.com/mrheinen/lophiid/actions/runs/10602859724/job/29385859494) [❌]

**Failure summary:** The action failed due to an error in fetching the 'org_golang_google_grpc' repository:

The fetch operation for 'google.golang.org/grpc@v1.63.2' timed out.

The error occurred while trying to connect to the proxy.golang.org server (IP: 142.250.190.17, port:
443).

This failure prevented the successful build of the '//pkg/backend/auth:auth' target, which depends
on the '@@org_golang_google_grpc//status:status' package.

The build process was aborted due to the analysis failure of the target '//pkg/backend/auth:auth'.

Relevant error logs:

```yaml 1: ##[group]Operating System 2: Ubuntu ... 574: [111 / 153] GoCompilePkg external/org_golang_google_protobuf/internal/impl/impl.a; 0s linux-sandbox ... (2 actions, 1 running) 575: Analyzing: 31 targets (221 packages loaded, 11332 targets configured) 576: [130 / 153] [Prepa] GoCompilePkg external/com_github_prometheus_client_golang/prometheus/testutil/promlint/promlint.a ... (2 actions, 0 running) 577: INFO: Repository org_golang_google_grpc instantiated at: 578: /home/runner/work/lophiid/lophiid/WORKSPACE:51:16: in 579: /home/runner/work/lophiid/lophiid/deps.bzl:1271:18: in go_dependencies 580: Repository rule go_repository defined at: 581: /home/runner/.cache/bazel/_bazel_runner/e4744c8e650cef2d9f0e7808187c3624/external/bazel_gazelle/internal/go_repository.bzl:340:32: in 582: ERROR: An error occurred during the fetch of repository 'org_golang_google_grpc': 583: Traceback (most recent call last): 584: File "/home/runner/.cache/bazel/_bazel_runner/e4744c8e650cef2d9f0e7808187c3624/external/bazel_gazelle/internal/go_repository.bzl", line 256, column 17, in _go_repository_impl 585: fail("failed to fetch %s: %s" % (ctx.name, result.stderr)) 586: Error in fail: failed to fetch org_golang_google_grpc: fetch_repo: google.golang.org/grpc@v1.63.2: Get "https://proxy.golang.org/google.golang.org/grpc/@v/v1.63.2.info": dial tcp 142.250.190.17:443: i/o timeout 587: ERROR: no such package '@@org_golang_google_grpc//status': failed to fetch org_golang_google_grpc: fetch_repo: google.golang.org/grpc@v1.63.2: Get "https://proxy.golang.org/google.golang.org/grpc/@v/v1.63.2.info": dial tcp 142.250.190.17:443: i/o timeout 588: ERROR: /home/runner/work/lophiid/lophiid/pkg/backend/auth/BUILD.bazel:3:11: //pkg/backend/auth:auth depends on @@org_golang_google_grpc//status:status in repository @@org_golang_google_grpc which failed to fetch. no such package '@@org_golang_google_grpc//status': failed to fetch org_golang_google_grpc: fetch_repo: google.golang.org/grpc@v1.63.2: Get "https://proxy.golang.org/google.golang.org/grpc/@v/v1.63.2.info": dial tcp 142.250.190.17:443: i/o timeout 589: Analyzing: 31 targets (221 packages loaded, 11332 targets configured) 590: [153 / 153] checking cached actions 591: ERROR: Analysis of target '//pkg/backend/auth:auth' failed; build aborted: Analysis failed 592: INFO: Elapsed time: 59.836s, Critical Path: 42.59s 593: INFO: 153 processes: 27 internal, 126 linux-sandbox. 594: ERROR: Build did NOT complete successfully 595: ##[error]Process completed with exit code 1. ```

✨ CI feedback usage guide:

The CI feedback tool (`/checks)` automatically triggers when a PR has a failed check. The tool analyzes the failed checks and provides several feedbacks: - Failed stage - Failed test name - Failure summary - Relevant error logs In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: ``` /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}" ``` where `{repo_name}` is the name of the repository, `{run_number}` is the run number of the failed check, and `{job_number}` is the job number of the failed check. #### Configuration options - `enable_auto_checks_feedback` - if set to true, the tool will automatically provide feedback when a check is failed. Default is true. - `excluded_checks_list` - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list. - `enable_help_text` - if set to true, the tool will provide a help message with the feedback. Default is true. - `persistent_comment` - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true. - `final_update_message` - if `persistent_comment` is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true. See more information about the `checks` tool in the [docs](https://pr-agent-docs.codium.ai/tools/ci_feedback/).

mrheinen / lophiid