Skip to content

Conversation

@uddhavdave
Copy link
Contributor

@uddhavdave uddhavdave commented Oct 14, 2025

Adds a config variable to control the length of hash
UI changes to accomodate Hash pattern policy

image image (3)

@github-actions
Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Validation Consistency

The new RePattern.hash_length is validated (12..64) but no defaulting occurs when out-of-range, unlike other checks that coerce values. Confirm that hard errors (panic on init) are the intended behavior and that docs/UI reflect this to avoid unexpected startup failures.

fn check_re_pattern_config(cfg: &mut Config) -> Result<(), anyhow::Error> {
    // Ensure hash_length is within valid range (minimum 12, maximum 64)
    if cfg.re_pattern.hash_length < 12 {
        return Err(anyhow::anyhow!(
            "ZO_RE_PATTERN_HASH_LENGTH must be at least 12 characters (configured: {})",
            cfg.re_pattern.hash_length
        ));
    }
    if cfg.re_pattern.hash_length > 64 {
        return Err(anyhow::anyhow!(
            "ZO_RE_PATTERN_HASH_LENGTH must be at most 64 characters (configured: {})",
            cfg.re_pattern.hash_length
        ));
    }
    Ok(())
}
String Mapping Robustness

PatternPolicy::from silently defaults to Redact for unrecognized strings. Consider logging or returning an error to make misconfiguration visible, especially with the new Hash option.

where
    T: AsRef<str>,
{
    fn from(value: T) -> Self {
        match value.as_ref() {
            "DropField" => Self::DropField,
            "Redact" => Self::Redact,
            "Hash" => Self::Hash,
            _ => Self::Redact,
        }
    }
Backward Compatibility

API now accepts optional policy and defaults to Redact. Ensure downstream behavior is identical to previous default paths and that enterprise feature-gated code handles missing/invalid policies deterministically.

pub async fn test(body: web::Bytes) -> Result<HttpResponse, Error> {
    #[cfg(feature = "enterprise")]
    {
        use infra::table::re_pattern_stream_map::PatternPolicy;
        use o2_enterprise::enterprise::re_patterns::PatternManager;

        let req: PatternTestRequest = match serde_json::from_slice(&body) {
            Ok(v) => v,
            Err(e) => return Ok(MetaHttpResponse::bad_request(e)),
        };

        let pattern = req.pattern;
        let inputs = req.test_records;
        // Default to Redact if policy not specified for backward compatibility
        let policy = req.policy.as_ref().map(|p| p.as_str()).unwrap_or("Redact");
        let policy = PatternPolicy::from(policy);

        let mut ret = Vec::with_capacity(inputs.len());
        for i in inputs {
            match PatternManager::test_pattern(pattern.clone(), i, policy) {
                Ok(v) => {
                    ret.push(v);
                }
                Err(e) => {

@github-actions
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Normalize policy string parsing

Make policy parsing case-insensitive to avoid silent fallback when the UI or API
sends different casing (e.g., "hash", "redact"). Normalize the input before
matching.

src/infra/src/table/re_pattern_stream_map.rs [53-63]

 impl<T> From<T> for PatternPolicy
 where
     T: AsRef<str>,
 {
     fn from(value: T) -> Self {
-        match value.as_ref() {
-            "DropField" => Self::DropField,
-            "Redact" => Self::Redact,
-            "Hash" => Self::Hash,
+        match value.as_ref().to_ascii_lowercase().as_str() {
+            "dropfield" => Self::DropField,
+            "redact" => Self::Redact,
+            "hash" => Self::Hash,
             _ => Self::Redact,
         }
     }
 }
Suggestion importance[1-10]: 7

__

Why: The change is correct and improves resilience by handling varied casing, directly aligned with the existing matcher; moderate positive impact without breaking behavior.

Medium
Sanitize policy input

Trim and normalize policy input to reduce user input errors (extra spaces, casing)
before converting to PatternPolicy. This prevents unexpected fallback behavior.

src/handler/http/request/re_pattern/mod.rs [456-461]

 let pattern = req.pattern;
 let inputs = req.test_records;
 // Default to Redact if policy not specified for backward compatibility
-let policy = req.policy.as_ref().map(|p| p.as_str()).unwrap_or("Redact");
-let policy = PatternPolicy::from(policy);
+let policy_str = req
+    .policy
+    .as_deref()
+    .map(|p| p.trim())
+    .filter(|p| !p.is_empty())
+    .unwrap_or("Redact");
+let policy = PatternPolicy::from(policy_str);
Suggestion importance[1-10]: 7

__

Why: This accurately targets the new policy handling code and reduces user input errors by trimming and defaulting safely; a clear, low-risk robustness improvement.

Medium
Possible issue
Clamp invalid config values

Avoid hard failing on out-of-range hash_length to prevent startup crashes due to
misconfiguration. Clamp the value to the valid range and log a warning so the
service remains available.

src/config/src/config.rs [2998-3013]

 fn check_re_pattern_config(cfg: &mut Config) -> Result<(), anyhow::Error> {
-    // Ensure hash_length is within valid range (minimum 12, maximum 64)
+    // Clamp hash_length to [12, 64] and warn instead of erroring
     if cfg.re_pattern.hash_length < 12 {
-        return Err(anyhow::anyhow!(
-            "ZO_RE_PATTERN_HASH_LENGTH must be at least 12 characters (configured: {})",
+        log::warn!(
+            "ZO_RE_PATTERN_HASH_LENGTH too small ({}). Using minimum 12.",
             cfg.re_pattern.hash_length
-        ));
-    }
-    if cfg.re_pattern.hash_length > 64 {
-        return Err(anyhow::anyhow!(
-            "ZO_RE_PATTERN_HASH_LENGTH must be at most 64 characters (configured: {})",
+        );
+        cfg.re_pattern.hash_length = 12;
+    } else if cfg.re_pattern.hash_length > 64 {
+        log::warn!(
+            "ZO_RE_PATTERN_HASH_LENGTH too large ({}). Using maximum 64.",
             cfg.re_pattern.hash_length
-        ));
+        );
+        cfg.re_pattern.hash_length = 64;
     }
     Ok(())
 }
Suggestion importance[1-10]: 6

__

Why: Suggestion is accurate and maps to the new check_re_pattern_config function lines, proposing a graceful clamp with warnings instead of panicking. It improves robustness but changes intended strict validation semantics, so impact is moderate.

Low

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Summary

This PR introduces a new Hash policy option for regex pattern matching in SDR (Sensitive Data Redaction), allowing matched values to be replaced with searchable hash prefixes instead of redacting or dropping them.

Key Changes:

  • Added ZO_RE_PATTERN_HASH_LENGTH configuration variable (default: 12, range: 12-64 characters) with validation
  • Extended PatternPolicy enum with Hash variant alongside existing Redact and DropField options
  • Updated pattern test endpoint to accept optional policy parameter, maintaining backward compatibility by defaulting to "Redact"
  • Added UI radio button for Hash policy with description "Replace with searchable hash"
  • Properly threaded policy parameter from frontend through API to backend handler

Architecture:
The implementation follows a clean layered approach: config validation at startup, enum extension in the data model, optional parameter handling in the API with sensible defaults, and UI updates. The actual hashing logic resides in the enterprise PatternManager component (not visible in this PR).

Confidence Score: 5/5

  • This PR is safe to merge with minimal risk
  • The changes are well-structured and follow defensive coding practices: config validation with clear bounds, backward-compatible API changes with sensible defaults, proper enum extension with serialization support, and clean UI integration. No breaking changes are introduced.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
src/config/src/config.rs 5/5 Added RePattern config struct with hash_length parameter (default 12, range 12-64) and validation logic
src/handler/http/request/re_pattern/mod.rs 5/5 Added optional policy parameter to test endpoint, defaults to "Redact" for backward compatibility
src/infra/src/table/re_pattern_stream_map.rs 5/5 Added Hash variant to PatternPolicy enum with proper serialization support
web/src/components/logstream/AssociatedRegexPatterns.vue 5/5 Added Hash radio button option to UI with description "Replace with searchable hash", passes policy to test function
web/src/services/regex_pattern.ts 5/5 Added optional policy parameter to test function, conditionally includes it in payload

Sequence Diagram

sequenceDiagram
    participant User
    participant UI as Vue Component
    participant API as regex_pattern.ts
    participant Handler as re_pattern/mod.rs
    participant PM as PatternManager (Enterprise)
    participant Config as Config System
    
    Note over Config: ZO_RE_PATTERN_HASH_LENGTH<br/>default: 12, range: 12-64
    
    User->>UI: Select Hash policy
    User->>UI: Enter test pattern & records
    UI->>API: test(org, pattern, records, "Hash")
    API->>Handler: POST /api/{org}/re_patterns/test
    Note over API: payload: {pattern, test_records, policy: "Hash"}
    Handler->>Handler: Parse policy (default "Redact")
    Handler->>Handler: Convert to PatternPolicy::Hash
    Handler->>PM: test_pattern(pattern, input, Hash)
    Note over PM: Uses config.re_pattern.hash_length<br/>to generate hash prefix
    PM-->>Handler: Hashed result
    Handler-->>API: PatternTestResponse
    API-->>UI: Display hashed output
    UI-->>User: Show result with searchable hash
Loading

5 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: udev | Branch: ud/feat-sdr-hash | Commit: 313fe3e

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 335 0 19 10 92% 5m 1s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: udev | Branch: ud/feat-sdr-hash | Commit: 313fe3e

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 363 330 0 19 14 91% 5m 0s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: udev | Branch: ud/feat-sdr-hash | Commit: 313fe3e

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 331 0 19 14 91% 5m 0s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: udev | Branch: ud/feat-sdr-hash | Commit: ba02411

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 337 0 19 8 93% 4m 37s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: uddhavdave | Branch: ud/feat-sdr-hash | Commit: c1da02e

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 338 0 19 7 93% 4m 40s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: uddhavdave | Branch: ud/feat-sdr-hash | Commit: dbdc3f3

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 335 0 19 10 92% 4m 55s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: uddhavdave | Branch: ud/feat-sdr-hash | Commit: e472d57

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 334 0 19 11 92% 5m 12s

View Detailed Results

@testdino-playwright-reporter
Copy link

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 345 0 19 0 95% 4m 40s

View Detailed Results

@testdino-playwright-reporter
Copy link

⚠️ Test Run Unstable


Author: uddhavdave | Branch: ud/feat-sdr-hash | Commit: 60081b4

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 342 0 19 3 94% 4m 39s

View Detailed Results

@testdino-playwright-reporter
Copy link

Testdino Test Results

Status Total Passed Failed Skipped Flaky Pass Rate Duration
All tests passed 364 345 0 19 0 95% 4m 38s

View Detailed Results

@uddhavdave uddhavdave merged commit 0c1268a into main Oct 17, 2025
34 of 40 checks passed
@uddhavdave uddhavdave deleted the ud/feat-sdr-hash branch October 17, 2025 11:32
uddhavdave added a commit that referenced this pull request Oct 27, 2025
Adds a config variable to control the length of hash
UI changes to accomodate Hash pattern policy

<img width="720" height="410" alt="image"
src="https://github.com/user-attachments/assets/b99e28ec-2b7d-431e-9606-7b855df62f8d"
/>

<img width="2037" height="1112" alt="image (3)"
src="https://github.com/user-attachments/assets/67432a01-c598-4b0a-830b-137a4cd04bf9"
/>

---------

Co-authored-by: Shrinath Rao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants