Skip to content

rule/filter initial definition load - #2687

Merged
osmontero merged 5 commits into
v11from
backlog/v11_rule/filter_sync
Sep 24, 2026
Merged

osmontero merged 5 commits into
v11from
backlog/v11_rule/filter_sync

Conversation

@AlexSanchez-bit

Copy link
Copy Markdown
Contributor

Main changes

  • added validations to rules and filters on startup sync

@AlexSanchez-bit
AlexSanchez-bit requested a review from a team September 24, 2026 15:28
@github-actions

github-actions Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

🛑 AI review — Engineer review required

This PR touches critical paths or introduces changes the model cannot judge with sufficient confidence. @Kbayero @osmontero please review.

✅ architecture (silas-1.7-pro) — clean

Summary: No architectural deviations; local improvements replace regex YAML parsing and add validation without touching agent, contracts, migrations, or critical paths.

No findings.

🛑 bugs (silas-1.7-pro) — blocking — must fix before merge

Summary: New filter sync uses unsafe SnakeYAML load and adds final Validator without visible constructor; missing trim/type checks can cause incorrect lookups.

  • high backend/src/main/java/com/park/utmstack/service/DefinitionSyncService.java:86 — yaml.load(content) uses SnakeYAML's default constructor, which can instantiate arbitrary Java objects if a filter file contains YAML tags. If filtersPath is writable by untrusted users, this is RCE. Use new Yaml(new SafeConstructor()) or parse into an expected safe type.
  • medium backend/src/main/java/com/park/utmstack/service/DefinitionSyncService.java:48 — Adding private final Validator validator; with no visible constructor update will not compile unless a generated constructor (e.g., Lombok) exists. Inject Validator in the constructor or use a non-final autowired field.
  • low backend/src/main/java/com/park/utmstack/service/DefinitionSyncService.java:86 — The old regex path trimmed the extracted dataType, but extractFirstDataType returns the raw scalar. A YAML value with surrounding spaces will not match the stored dataType after toLowerCase(). Trim the returned string.
  • low backend/src/main/java/com/park/utmstack/service/DefinitionSyncService.java:295 — extractFirstDataType returns toString() for any non-null first dataTypes element, including Map/List. Validate that the element is a scalar or coerce it safely; otherwise malformed nested YAML produces an invalid dataType string.

🛑 security (silas-1.7-pro) — blocking — must fix before merge

Summary: PR introduces unsafe SnakeYAML 1.33 parsing of filter files, enabling unsafe deserialization/RCE if untrusted YAML is present.

  • high backend/pom.xml:18 — SnakeYAML is pinned to 1.33, which is vulnerable to unsafe deserialization/RCE (CVE-2022-1471). Upgrade to 2.2+ and ensure callers use SafeConstructor.
  • high backend/src/main/java/com/park/utmstack/service/DefinitionSyncService.java:86 — yaml.load(content) uses the default SnakeYAML constructor, allowing arbitrary object instantiation if a malicious YAML filter is processed. Use new Yaml(new SafeConstructor()) or restrict parsing to known types.

@utmstackprapprover utmstackprapprover Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested — Go dependencies check failed (see above).

@AlexSanchez-bit AlexSanchez-bit linked an issue Sep 24, 2026 that may be closed by this pull request
3 tasks
@AlexSanchez-bit

Copy link
Copy Markdown
Contributor Author

founded roules is used to clean orphaned sytem rules at startup (remove rules that are in database but not in filesystem) a system rule may be edited and have wrong structure, that doesnt mean it must be deleted from the database, so adding rule to founded even if they have wrong structure at the time makes sense.

@AlexSanchez-bit

AlexSanchez-bit commented Sep 24, 2026 •

Copy link
Copy Markdown
Contributor Author

springboot dependency injection system will inject the validator as this class has RequiredArgsConstructor decorator which will automatically declare the needed constructor

@utmstackprapprover utmstackprapprover Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved — no blocking issues, deps OK, authorized author. Any non-blocking warnings are listed above.

@utmstackprapprover utmstackprapprover Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested — AI review found blocking issues (high/critical, or engineer review required). See above.

@AlexSanchez-bit

Copy link
Copy Markdown
Contributor Author

yaml parsed files are internal, and 1.33 is the latest compatible version of the dependency

@osmontero
osmontero merged commit 3872910 into v11 Sep 24, 2026
2 checks passed
@osmontero
osmontero deleted the backlog/v11_rule/filter_sync branch September 24, 2026 17:12
@github-actions

Copy link
Copy Markdown

❌ Go dependencies check failed

There are outdated Go dependencies, or modules that could not be inspected.
Run bash .github/scripts/go-deps.sh --update --discover locally and
commit the updated go.mod / go.sum files.

Script output
🔍 Discovered 25 Go projects

📦 Dependencies with updates available:

  📁 ./utmstack-collector:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/gcp:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/aws:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/alerts:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/events:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/inputs:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/stats:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/rule-flood-guard:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/o365:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/modules-config:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/config:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/soc-ai:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/sophos:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/azure:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/crowdstrike:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/bitdefender:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./plugins/geolocation:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./agent-manager:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./agent:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./as400:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

  📁 ./as400/updater:
     - github.com/threatwinds/go-sdk: v1.1.34 → v1.1.35

�[0;31m❌ Please update dependencies before merging.�[0m

@utmstackprapprover utmstackprapprover Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested — Go dependencies check failed (see above).

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.

rules, filter definition not validating structure at startup

2 participants