carverauto/serviceradar
1
1
Fork 0
Code Issues 190 Pull requests 16 Projects Releases 173 Packages Wiki Activity Actions

2057 featsysmon integrate edge onboarding package #2507

Merged
mfreeman451 merged 4 commits from refs/pull/2507/head into main 2025-12-04 19:35:25 +00:00
Conversation 2 Commits 4 Files changed 28 +2524 -12
mfreeman451 commented 2025-12-04 18:57:58 +00:00 (Migrated from github.com)
Owner
Copy link

Imported from GitHub pull request.

Original GitHub pull request: #2059
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/2059
Original created: 2025-12-04T18:57:58Z
Original updated: 2025-12-04T19:35:28Z
Original head: carverauto/serviceradar:2057-featsysmon-integrate-edge-onboarding-package
Original base: main
Original merged: 2025-12-04T19:35:25Z by @mfreeman451

User description

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Issue ticket number and link

Code checklist before requesting a review

  • I have signed the DCO?
  • The build completes without errors?
  • All tests are passing when running make test?

PR Type

Enhancement

Description

  • Create new edge-onboarding Rust crate mirroring Go package functionality

    • Token parsing and validation for edgepkg-v1: format
    • Package download from Core API with mTLS bundle support
    • Certificate installation with proper file permissions
    • Deployment type detection (Docker, Kubernetes, bare-metal)
    • Configuration generation for sysmon checker

  • Integrate edge onboarding into sysmon checker with CLI and environment support

    • Add --mtls, --token, --host, --bundle, --cert-dir CLI flags
    • Support ONBOARDING_TOKEN, CORE_API_URL environment variables
    • Fallback to traditional config when no onboarding token provided

  • Add comprehensive test coverage with 14 unit tests across all modules

  • Update Docker image with edge onboarding directories and environment variables

  • Document edge onboarding usage in README and design specifications

Diagram Walkthrough

flowchart LR
  A["Admin UI<br/>generates token"] -->|edgepkg-v1| B["sysmon checker<br/>CLI/env"]
  B -->|--mtls flag| C["edge-onboarding<br/>crate"]
  C -->|parse_token| D["Token validation"]
  D -->|download_package| E["Core API<br/>package download"]
  E -->|mTLS bundle| F["install_mtls_bundle"]
  F -->|certs + config| G["Generated config<br/>+ credentials"]
  G -->|persisted| H["gRPC server<br/>with mTLS"]

File Walkthrough
Relevant files
Enhancement
7 files
lib.rs
Main edge onboarding library with public API                         
+346/-0 
token.rs
Token parsing for edgepkg-v1 format                                           
+156/-0 
download.rs
Core API package download functionality                                   
+143/-0 
bundle.rs
mTLS bundle installation and loading                                         
+196/-0 
config.rs
Configuration generation for sysmon checker                           
+180/-0 
deployment.rs
Deployment type detection logic                                                   
+99/-0   
main.rs
Integrate edge onboarding with CLI and env support             
+99/-3   
Error handling
1 files
error.rs
Error types for edge onboarding operations                             
+86/-0   
Dependencies
2 files
Cargo.toml
New crate manifest with dependencies                                         
+22/-0   
Cargo.toml
Add edge-onboarding dependency                                                     
+1/-0     
Tests
6 files
token_tests.rs
Comprehensive token parsing unit tests                                     
+216/-0 
bundle_tests.rs
mTLS bundle installation tests                                                     
+78/-0   
config_tests.rs
Configuration generation tests                                                     
+91/-0   
deployment_tests.rs
Deployment type detection tests                                                   
+29/-0   
download_tests.rs
Package download validation tests                                               
+49/-0   
lib_tests.rs
Integration tests for public API                                                 
+119/-0 
Configuration changes
2 files
Dockerfile
Add edge onboarding directories and environment variables
 +31/-2   
Cargo.toml
Add edge-onboarding to workspace members                                 
+1/-0     
Documentation
6 files
README.md
Document edge onboarding usage and CLI options                     
+51/-0   
AGENTS.md
Add edge onboarding testing playbook with Docker Compose 
+133/-1 
proposal.md
Change proposal and status documentation                                 
+45/-0   
design.md
Detailed design and architecture documentation                     
+126/-0 
spec.md
Formal requirements and scenarios specification                   
+65/-0   
tasks.md
Implementation tasks and validation checklist                       
+60/-0   
Imported from GitHub pull request. Original GitHub pull request: #2059 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/2059 Original created: 2025-12-04T18:57:58Z Original updated: 2025-12-04T19:35:28Z Original head: carverauto/serviceradar:2057-featsysmon-integrate-edge-onboarding-package Original base: main Original merged: 2025-12-04T19:35:25Z by @mfreeman451 --- ### **User description** ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test? ___ ### **PR Type** Enhancement ___ ### **Description** - Create new `edge-onboarding` Rust crate mirroring Go package functionality - Token parsing and validation for `edgepkg-v1:` format - Package download from Core API with mTLS bundle support - Certificate installation with proper file permissions - Deployment type detection (Docker, Kubernetes, bare-metal) - Configuration generation for sysmon checker - Integrate edge onboarding into sysmon checker with CLI and environment support - Add `--mtls`, `--token`, `--host`, `--bundle`, `--cert-dir` CLI flags - Support `ONBOARDING_TOKEN`, `CORE_API_URL` environment variables - Fallback to traditional config when no onboarding token provided - Add comprehensive test coverage with 14 unit tests across all modules - Update Docker image with edge onboarding directories and environment variables - Document edge onboarding usage in README and design specifications ___ ### Diagram Walkthrough ```mermaid flowchart LR A["Admin UI<br/>generates token"] -->|edgepkg-v1| B["sysmon checker<br/>CLI/env"] B -->|--mtls flag| C["edge-onboarding<br/>crate"] C -->|parse_token| D["Token validation"] D -->|download_package| E["Core API<br/>package download"] E -->|mTLS bundle| F["install_mtls_bundle"] F -->|certs + config| G["Generated config<br/>+ credentials"] G -->|persisted| H["gRPC server<br/>with mTLS"] ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><details><summary>7 files</summary><table> <tr> <td><strong>lib.rs</strong><dd><code>Main edge onboarding library with public API</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-5d04060c8999c31f6f944993ba5c30a1d7cc1882441105b20e6aae0aa0c2eb52">+346/-0</a>&nbsp; </td> </tr> <tr> <td><strong>token.rs</strong><dd><code>Token parsing for edgepkg-v1 format</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-522ddb82ad9919bd416394e34bbfd4ce3e8793723b260a08843d7500ae0b98cd">+156/-0</a>&nbsp; </td> </tr> <tr> <td><strong>download.rs</strong><dd><code>Core API package download functionality</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-9e5dc37d31894894a78d7eb118c72f7303a8be99adb992ac51de9e664fdee2f2">+143/-0</a>&nbsp; </td> </tr> <tr> <td><strong>bundle.rs</strong><dd><code>mTLS bundle installation and loading</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-fc09c2867b39b36b1463f053656aaad70a259979ef64a00fcb4130a437d4ef3d">+196/-0</a>&nbsp; </td> </tr> <tr> <td><strong>config.rs</strong><dd><code>Configuration generation for sysmon checker</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-27bc48513a0a6f736d1b6efe62b5eb7dcfccc82e70f4cb9678d54ee1efe9455d">+180/-0</a>&nbsp; </td> </tr> <tr> <td><strong>deployment.rs</strong><dd><code>Deployment type detection logic</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-6e3f14380f006c4e9e1dd68a6ddc857dfa736987ffcf21e3772a966e664f58f2">+99/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>main.rs</strong><dd><code>Integrate edge onboarding with CLI and env support</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-cef134403622089f759cfa006eb04dc8f1cfda08497a9d38586cc4de03d14820">+99/-3</a>&nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Error handling</strong></td><td><details><summary>1 files</summary><table> <tr> <td><strong>error.rs</strong><dd><code>Error types for edge onboarding operations</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-89b462756c4e0679e6d1ac925ed2f65650b07646415bd8e202ff235ca18937d5">+86/-0</a>&nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Dependencies</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>Cargo.toml</strong><dd><code>New crate manifest with dependencies</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-a642d44a71cf5973a88eab24508c6e2de618ffaf8afd0d6124013ed7e7f70ca7">+22/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>Cargo.toml</strong><dd><code>Add edge-onboarding dependency</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-6f8e9ee62fdd4b2de3f99c8760f26dcb7c0b1e6b83ade0a888898d23f6abcbf3">+1/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Tests</strong></td><td><details><summary>6 files</summary><table> <tr> <td><strong>token_tests.rs</strong><dd><code>Comprehensive token parsing unit tests</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-5c3192711c7e59713634180745b00fa6ac48caff6d9fbe5392b357362ace1761">+216/-0</a>&nbsp; </td> </tr> <tr> <td><strong>bundle_tests.rs</strong><dd><code>mTLS bundle installation tests</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-355d1522f2d20692ba71c9c910d3cbb668f871f8ce026ae6ee4bd30e3719db0e">+78/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>config_tests.rs</strong><dd><code>Configuration generation tests</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-f8e16ab47f62bbda945d5b9b5bd6940827e75202258079c5d5909c3124761e3a">+91/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>deployment_tests.rs</strong><dd><code>Deployment type detection tests</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-5c0dddde3864af912201e90e9215cde114ca30aa02ec2aebcaf45e458092a68c">+29/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>download_tests.rs</strong><dd><code>Package download validation tests</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-af4cc73cc6503308a1066a090954c5e23b341524d66944e950e64a215d8a2c3a">+49/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>lib_tests.rs</strong><dd><code>Integration tests for public API</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-82c278e0226666fd2188807f98c5df6612bf7425c723f6332cea94a0abb859f1">+119/-0</a>&nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Configuration changes</strong></td><td><details><summary>2 files</summary><table> <tr> <td><strong>Dockerfile</strong><dd><code>Add edge onboarding directories and environment variables</code></dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-e89e6766c5fa16966c84ef0be4ca5ec55560150ef5583a605e5e55a31a08befb">+31/-2</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>Cargo.toml</strong><dd><code>Add edge-onboarding to workspace members</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542">+1/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></details></td></tr><tr><td><strong>Documentation</strong></td><td><details><summary>6 files</summary><table> <tr> <td><strong>README.md</strong><dd><code>Document edge onboarding usage and CLI options</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-57ebcc9bd94d5a042ca261b795e1d851fddb9f3051461c2d82747287eba49bca">+51/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>AGENTS.md</strong><dd><code>Add edge onboarding testing playbook with Docker Compose</code>&nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-a54ff182c7e8acf56acfd6e4b9c3ff41e2c41a31c9b211b2deb9df75d9a478f9">+133/-1</a>&nbsp; </td> </tr> <tr> <td><strong>proposal.md</strong><dd><code>Change proposal and status documentation</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-5eec4c44ee16c05009a335e42f8d1f2f51f2141a67b8ecaf4e10bb89750aaf01">+45/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>design.md</strong><dd><code>Detailed design and architecture documentation</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-5274bb4e518de9acae67adc9f953dff53e206dd3c1c74b1f7d31aa6a180a3657">+126/-0</a>&nbsp; </td> </tr> <tr> <td><strong>spec.md</strong><dd><code>Formal requirements and scenarios specification</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-13e8b130ad04fa345823dca1c00eaab10c696e215a5f103e81bb9be68a3a9c0d">+65/-0</a>&nbsp; &nbsp; </td> </tr> <tr> <td><strong>tasks.md</strong><dd><code>Implementation tasks and validation checklist</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></td> <td><a href="https://github.com/carverauto/serviceradar/pull/2059/files#diff-da976ef6be4f59ef694d7366995b6cc21249f4250cde4d376cd22217a0d8efae">+60/-0</a>&nbsp; &nbsp; </td> </tr> </table></details></td></tr></tr></tbody></table> </details> ___
qodo-code-review[bot] commented 2025-12-04 18:58:57 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2059#issuecomment-3613888712
Original created: 2025-12-04T18:58:57Z

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
⚪
Insecure transport fallback

Description: HTTP client errors from Core API (ureq::Error::Transport) are converted to string without
nuanced handling (e.g., timeouts, TLS validation), and ensure_scheme defaults to http://
when no scheme is provided, which could lead to cleartext requests if the token omits
scheme.
download.rs [100-118]

Referred Code 

tracing::debug!(url = %url, package_id = %payload.package_id, "Downloading package from Core API");

let response = ureq::post(&url)
    .set("Content-Type", "application/json")
    .set("Accept", "application/json")
    .send_json(&request_body)
    .map_err(|e| match e {
        ureq::Error::Status(status, resp) => {
            let body = resp.into_string().unwrap_or_default();
            Error::CoreApiError {
                status,
                message: body,
            }
        }
        ureq::Error::Transport(t) => Error::Http(t.to_string()),
    })?;

let package: PackageResponse = response.into_json().map_err(|e| Error::Http(e.to_string()))?;
Unsafe file writes

Description: Certificate and key files are written from untrusted bundle content without validation of
PEM format or restricting file ownership, potentially enabling misconfiguration or
privilege issues if paths are attacker-controlled.
bundle.rs [183-196]

Referred Code 
fn write_file(path: &Path, content: &str, mode: u32) -> Result<()> {
    fs::write(path, content).map_err(|e| Error::Io {
        path: path.display().to_string(),
        source: e,
    })?;

    let perms = fs::Permissions::from_mode(mode);
    fs::set_permissions(path, perms).map_err(|e| Error::Io {
        path: path.display().to_string(),
        source: e,
    })?;

    Ok(())
}
Ticket Compliance
🟡
🎫 #2057
🟢 Integrate existing edge onboarding functionality into the Rust sysmon package to match
sysmon-osx capabilities.
Provide token parsing and validation for edgepkg-v1 format.
Download onboarding package from Core API including mTLS bundle support.
Install certificates with proper file permissions during mTLS onboarding.
Detect deployment type (Docker, Kubernetes, bare-metal) and use appropriate paths.
Generate configuration for the sysmon checker based on onboarding package and security
mode.
Integrate CLI and environment-based onboarding into sysmon with flags (--mtls, --token,
--host, --bundle, --cert-dir) and env vars (ONBOARDING_TOKEN, CORE_API_URL).
Fallback to traditional config when no onboarding token is provided.
Add comprehensive tests covering modules (unit tests).
Update Docker image to include edge onboarding directories and environment variables;
support onboarding at runtime.
Document edge onboarding usage in README and design/spec documents.
⚪ End-to-end verification against a live Core API and poller that mTLS connections succeed
with generated certs.
Validation of Kubernetes deployment behavior and paths in real cluster environments.
Confirmation that permissions on installed cert/key files are honored across different
host filesystems and container runtimes.
Codebase Duplication Compliance
⚪
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Token exposure risk: The code accepts onboarding tokens via --token and environment but logs onboarding success
and SPIFFE ID without masking inputs; combined with other logs and error paths there is no
explicit redaction or guarantee that sensitive tokens or bundle contents are not logged.

Referred Code 
// Try environment-based edge onboarding if no config path yet
if config_path.is_none() {
    match edge_onboarding::try_onboard(ComponentType::Checker) {
        Ok(Some(result)) => {
            info!("Edge onboarding successful");
            info!("Generated config at: {}", result.config_path);
            if let Some(ref spiffe_id) = result.spiffe_id {
                info!("SPIFFE ID: {}", spiffe_id);
            }
            config_path = Some(PathBuf::from(&result.config_path));
        }
        Ok(None) => {
            // No onboarding token, need a config file
            return Err(anyhow::anyhow!(
                "No configuration provided. Use --config, --mtls, or set ONBOARDING_TOKEN"
            ));
        }
        Err(e) => {
            return Err(anyhow::anyhow!("Edge onboarding failed: {}", e));
        }
    }

Learn more about managing compliance generic rules or creating your own custom rules

⚪
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Action logging: New onboarding flows perform critical actions (token use, package download, cert install,
config generation) but rely on info! logs without structured fields for user ID or full
action context, which may be insufficient for audit trail requirements.

Referred Code 
// Try mTLS bootstrap first if --mtls flag is set
if mtls_mode {
    let mtls_token = token.clone().or_else(|| std::env::var("ONBOARDING_TOKEN").ok());
    if let Some(t) = mtls_token {
        info!("mTLS bootstrap mode enabled");
        let mtls_config = MtlsBootstrapConfig {
            token: t,
            host,
            bundle_path,
            cert_dir,
            service_name: Some("sysmon".to_string()),
        };

        match edge_onboarding::mtls_bootstrap(&mtls_config) {
            Ok(result) => {
                info!("mTLS bootstrap successful");
                info!("Generated config at: {}", result.config_path);
                info!("Certificates installed to: {}", result.cert_dir);
                config_path = Some(PathBuf::from(&result.config_path));
            }
            Err(e) => {


 ... (clipped 31 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Error detail exposure: Core API HTTP errors propagate status and response body directly into errors which may be
surfaced to users, potentially exposing internal details.

Referred Code 

tracing::debug!(url = %url, package_id = %payload.package_id, "Downloading package from Core API");

let response = ureq::post(&url)
    .set("Content-Type", "application/json")
    .set("Accept", "application/json")
    .send_json(&request_body)
    .map_err(|e| match e {
        ureq::Error::Status(status, resp) => {
            let body = resp.into_string().unwrap_or_default();
            Error::CoreApiError {
                status,
                message: body,
            }
        }
        ureq::Error::Transport(t) => Error::Http(t.to_string()),
    })?;

let package: PackageResponse = response.into_json().map_err(|e| Error::Http(e.to_string()))?;

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
URL handling: The ensure_scheme helper auto-prefixes http:// to host values which may unintentionally
downgrade transport security if https is expected, requiring confirmation of intended
security posture.

Referred Code 
fn ensure_scheme(host: &str) -> Result<String> {
    let host = host.trim();
    if host.is_empty() {
        return Err(Error::CoreApiHostRequired);
    }
    if host.starts_with("http://") || host.starts_with("https://") {
        return Ok(host.to_string());
    }
    Ok(format!("http://{}", host))
}

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2059#issuecomment-3613888712 Original created: 2025-12-04T18:58:57Z --- ## PR Compliance Guide 🔍 <!-- https://github.com/carverauto/serviceradar/commit/2da1cfb42c43dcad3e84c21b6d08957186598db7 --> Below is a summary of compliance checks for this PR:<br> <table><tbody><tr><td colspan='2'><strong>Security Compliance</strong></td></tr> <tr><td rowspan=2>⚪</td> <td><details><summary><strong>Insecure transport fallback </strong></summary><br> <b>Description:</b> HTTP client errors from Core API (ureq::Error::Transport) are converted to string without <br>nuanced handling (e.g., timeouts, TLS validation), and ensure_scheme defaults to http:// <br>when no scheme is provided, which could lead to cleartext requests if the token omits <br>scheme.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/2059/files#diff-9e5dc37d31894894a78d7eb118c72f7303a8be99adb992ac51de9e664fdee2f2R100-R118'>download.rs [100-118]</a></strong><br> <details open><summary>Referred Code</summary> ```rust tracing::debug!(url = %url, package_id = %payload.package_id, "Downloading package from Core API"); let response = ureq::post(&url) .set("Content-Type", "application/json") .set("Accept", "application/json") .send_json(&request_body) .map_err(|e| match e { ureq::Error::Status(status, resp) => { let body = resp.into_string().unwrap_or_default(); Error::CoreApiError { status, message: body, } } ureq::Error::Transport(t) => Error::Http(t.to_string()), })?; let package: PackageResponse = response.into_json().map_err(|e| Error::Http(e.to_string()))?; ``` </details></details></td></tr> <tr><td><details><summary><strong>Unsafe file writes </strong></summary><br> <b>Description:</b> Certificate and key files are written from untrusted bundle content without validation of <br>PEM format or restricting file ownership, potentially enabling misconfiguration or <br>privilege issues if paths are attacker-controlled.<br> <strong><a href='https://github.com/carverauto/serviceradar/pull/2059/files#diff-fc09c2867b39b36b1463f053656aaad70a259979ef64a00fcb4130a437d4ef3dR183-R196'>bundle.rs [183-196]</a></strong><br> <details open><summary>Referred Code</summary> ```rust fn write_file(path: &Path, content: &str, mode: u32) -> Result<()> { fs::write(path, content).map_err(|e| Error::Io { path: path.display().to_string(), source: e, })?; let perms = fs::Permissions::from_mode(mode); fs::set_permissions(path, perms).map_err(|e| Error::Io { path: path.display().to_string(), source: e, })?; Ok(()) } ``` </details></details></td></tr> <tr><td colspan='2'><strong>Ticket Compliance</strong></td></tr> <tr><td>🟡</td> <td> <details> <summary>🎫 <a href=https://github.com/carverauto/serviceradar/issues/2057>#2057</a></summary> <table width='100%'><tbody> <tr><td rowspan=11>🟢</td> <td>Integrate existing edge onboarding functionality into the Rust sysmon package to match <br>sysmon-osx capabilities.</td></tr> <tr><td>Provide token parsing and validation for edgepkg-v1 format.</td></tr> <tr><td>Download onboarding package from Core API including mTLS bundle support.</td></tr> <tr><td>Install certificates with proper file permissions during mTLS onboarding.</td></tr> <tr><td>Detect deployment type (Docker, Kubernetes, bare-metal) and use appropriate paths.</td></tr> <tr><td>Generate configuration for the sysmon checker based on onboarding package and security <br>mode.</td></tr> <tr><td>Integrate CLI and environment-based onboarding into sysmon with flags (--mtls, --token, <br>--host, --bundle, --cert-dir) and env vars (ONBOARDING_TOKEN, CORE_API_URL).</td></tr> <tr><td>Fallback to traditional config when no onboarding token is provided.</td></tr> <tr><td>Add comprehensive tests covering modules (unit tests).</td></tr> <tr><td>Update Docker image to include edge onboarding directories and environment variables; <br>support onboarding at runtime.</td></tr> <tr><td>Document edge onboarding usage in README and design/spec documents.</td></tr> <tr><td rowspan=3>⚪</td> <td>End-to-end verification against a live Core API and poller that mTLS connections succeed <br>with generated certs.</td></tr> <tr><td>Validation of Kubernetes deployment behavior and paths in real cluster environments.</td></tr> <tr><td>Confirmation that permissions on installed cert/key files are honored across different <br>host filesystems and container runtimes.</td></tr> </tbody></table> </details> </td></tr> <tr><td colspan='2'><strong>Codebase Duplication Compliance</strong></td></tr> <tr><td>⚪</td><td><details><summary><strong>Codebase context is not defined </strong></summary> Follow the <a href='https://qodo-merge-docs.qodo.ai/core-abilities/rag_context_enrichment/'>guide</a> to enable codebase context checks. </details></td></tr> <tr><td colspan='2'><strong>Custom Compliance</strong></td></tr> <tr><td rowspan=2>🟢</td><td> <details><summary><strong>Generic: Meaningful Naming and Self-Documenting Code</strong></summary><br> **Objective:** Ensure all identifiers clearly express their purpose and intent, making code <br>self-documenting<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td> <details><summary><strong>Generic: Robust Error Handling and Edge Case Management</strong></summary><br> **Objective:** Ensure comprehensive error handling that provides meaningful context and graceful <br>degradation<br> **Status:** Passed<br> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=1>🔴</td> <td><details> <summary><strong>Generic: Secure Logging Practices</strong></summary><br> **Objective:** To ensure logs are useful for debugging and auditing without exposing sensitive <br>information like PII, PHI, or cardholder data.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2059/files#diff-cef134403622089f759cfa006eb04dc8f1cfda08497a9d38586cc4de03d14820R131-R151'><strong>Token exposure risk</strong></a>: The code accepts onboarding tokens via --token and environment but logs onboarding success <br>and SPIFFE ID without masking inputs; combined with other logs and error paths there is no <br>explicit redaction or guarantee that sensitive tokens or bundle contents are not logged.<br> <details open><summary>Referred Code</summary> ```rust // Try environment-based edge onboarding if no config path yet if config_path.is_none() { match edge_onboarding::try_onboard(ComponentType::Checker) { Ok(Some(result)) => { info!("Edge onboarding successful"); info!("Generated config at: {}", result.config_path); if let Some(ref spiffe_id) = result.spiffe_id { info!("SPIFFE ID: {}", spiffe_id); } config_path = Some(PathBuf::from(&result.config_path)); } Ok(None) => { // No onboarding token, need a config file return Err(anyhow::anyhow!( "No configuration provided. Use --config, --mtls, or set ONBOARDING_TOKEN" )); } Err(e) => { return Err(anyhow::anyhow!("Edge onboarding failed: {}", e)); } } ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td rowspan=3>⚪</td> <td><details> <summary><strong>Generic: Comprehensive Audit Trails</strong></summary><br> **Objective:** To create a detailed and reliable record of critical system actions for security analysis <br>and compliance.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2059/files#diff-cef134403622089f759cfa006eb04dc8f1cfda08497a9d38586cc4de03d14820R100-R151'><strong>Action logging</strong></a>: New onboarding flows perform critical actions (token use, package download, cert install, <br>config generation) but rely on info! logs without structured fields for user ID or full <br>action context, which may be insufficient for audit trail requirements.<br> <details open><summary>Referred Code</summary> ```rust // Try mTLS bootstrap first if --mtls flag is set if mtls_mode { let mtls_token = token.clone().or_else(|| std::env::var("ONBOARDING_TOKEN").ok()); if let Some(t) = mtls_token { info!("mTLS bootstrap mode enabled"); let mtls_config = MtlsBootstrapConfig { token: t, host, bundle_path, cert_dir, service_name: Some("sysmon".to_string()), }; match edge_onboarding::mtls_bootstrap(&mtls_config) { Ok(result) => { info!("mTLS bootstrap successful"); info!("Generated config at: {}", result.config_path); info!("Certificates installed to: {}", result.cert_dir); config_path = Some(PathBuf::from(&result.config_path)); } Err(e) => { ... (clipped 31 lines) ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Secure Error Handling</strong></summary><br> **Objective:** To prevent the leakage of sensitive system information through error messages while <br>providing sufficient detail for internal debugging.<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2059/files#diff-9e5dc37d31894894a78d7eb118c72f7303a8be99adb992ac51de9e664fdee2f2R100-R119'><strong>Error detail exposure</strong></a>: Core API HTTP errors propagate status and response body directly into errors which may be <br>surfaced to users, potentially exposing internal details.<br> <details open><summary>Referred Code</summary> ```rust tracing::debug!(url = %url, package_id = %payload.package_id, "Downloading package from Core API"); let response = ureq::post(&url) .set("Content-Type", "application/json") .set("Accept", "application/json") .send_json(&request_body) .map_err(|e| match e { ureq::Error::Status(status, resp) => { let body = resp.into_string().unwrap_or_default(); Error::CoreApiError { status, message: body, } } ureq::Error::Transport(t) => Error::Http(t.to_string()), })?; let package: PackageResponse = response.into_json().map_err(|e| Error::Http(e.to_string()))?; ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td><details> <summary><strong>Generic: Security-First Input Validation and Data Handling</strong></summary><br> **Objective:** Ensure all data inputs are validated, sanitized, and handled securely to prevent <br>vulnerabilities<br> **Status:** <br><a href='https://github.com/carverauto/serviceradar/pull/2059/files#diff-9e5dc37d31894894a78d7eb118c72f7303a8be99adb992ac51de9e664fdee2f2R134-R143'><strong>URL handling</strong></a>: The ensure_scheme helper auto-prefixes http:// to host values which may unintentionally <br>downgrade transport security if https is expected, requiring confirmation of intended <br>security posture.<br> <details open><summary>Referred Code</summary> ```rust fn ensure_scheme(host: &str) -> Result<String> { let host = host.trim(); if host.is_empty() { return Err(Error::CoreApiHostRequired); } if host.starts_with("http://") || host.starts_with("https://") { return Ok(host.to_string()); } Ok(format!("http://{}", host)) } ``` </details> > Learn more about managing compliance <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#configuration-options'>generic rules</a> or creating your own <a href='https://qodo-merge-docs.qodo.ai/tools/compliance/#custom-compliance'>custom rules</a> </details></td></tr> <tr><td align="center" colspan="2"> - [ ] Update <!-- /compliance --update_compliance=true --> </td></tr></tbody></table> <details><summary>Compliance status legend</summary> 🟢 - Fully Compliant<br> 🟡 - Partial Compliant<br> 🔴 - Not Compliant<br> ⚪ - Requires Further Human Verification<br> 🏷️ - Compliance label<br> </details>
qodo-code-review[bot] commented 2025-12-04 19:00:21 +00:00 (Migrated from github.com)
Author
Owner
Copy link

Imported GitHub PR comment.

Original author: @qodo-code-review[bot]
Original URL: https://github.com/carverauto/serviceradar/pull/2059#issuecomment-3613893651
Original created: 2025-12-04T19:00:21Z

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Default to HTTPS for better security

In the ensure_scheme function, change the default protocol to https:// instead
of http:// for better security. An exception can be made to use http:// for
localhost addresses.

rust/edge-onboarding/src/download.rs [133-143]

 /// Ensure the URL has a scheme (http:// or https://).
 fn ensure_scheme(host: &str) -> Result<String> {
     let host = host.trim();
     if host.is_empty() {
         return Err(Error::CoreApiHostRequired);
     }
     if host.starts_with("http://") || host.starts_with("https://") {
         return Ok(host.to_string());
     }
-    Ok(format!("http://{}", host))
+    // Default to https for security, but allow http for localhost.
+    if host.starts_with("localhost") || host.starts_with("127.0.0.1") {
+        Ok(format!("http://{}", host))
+    } else {
+        Ok(format!("https://{}", host))
+    }
 }
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies a security weakness where the client might communicate over unencrypted HTTP by default. Changing the default to HTTPS is a significant security improvement that aligns with secure-by-default principles.

Medium
General
Improve legacy token parsing logic

In parse_legacy_token, replace find with rfind when searching for separators.
This makes parsing more robust by splitting on the last occurrence of a
separator, which is a common pattern for id:token formats.

rust/edge-onboarding/src/token.rs [96-133]

 fn parse_legacy_token(
     raw: &str,
     fallback_package_id: Option<&str>,
     fallback_core_url: Option<&str>,
 ) -> Result<TokenPayload> {
     let mut payload = TokenPayload {
         package_id: fallback_package_id.unwrap_or_default().to_string(),
         download_token: String::new(),
         core_url: fallback_core_url.map(String::from),
     };
 
     let mut legacy = raw.to_string();
 
     // Check for URL@remainder format
     if let Some(at_idx) = legacy.find('@') {
         let maybe_url = legacy[..at_idx].trim();
         let remainder = legacy[at_idx + 1..].trim();
         if looks_like_url(maybe_url) && !remainder.is_empty() {
             payload.core_url = Some(maybe_url.to_string());
             legacy = remainder.to_string();
         }
     }
 
-    // Try to split by common separators
+    // Try to split by common separators, using the last occurrence
     for sep in [':', '/', '|', ','] {
-        if let Some(idx) = legacy.find(sep) {
+        if let Some(idx) = legacy.rfind(sep) {
             payload.package_id = legacy[..idx].trim().to_string();
             payload.download_token = legacy[idx + 1..].trim().to_string();
             validate_token_payload(&payload)?;
             return Ok(payload);
         }
     }
 
     // No separator found - treat entire string as download token
     payload.download_token = legacy.trim().to_string();
     validate_token_payload(&payload)?;
     Ok(payload)
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies a potential ambiguity in parsing legacy tokens and proposes using rfind to make the logic more robust. This is a good improvement for handling edge cases where separators might appear in the package ID.

Medium
Simplify function with redundant match arms

Simplify the get_storage_path function by removing the redundant match
statement, as all deployment types currently resolve to the same storage path.
The first argument can be marked as unused.

rust/edge-onboarding/src/lib.rs [313-326]

 /// Get storage path based on deployment type.
-fn get_storage_path(deployment_type: &DeploymentType, component_type: ComponentType) -> PathBuf {
-    match deployment_type {
-        DeploymentType::Docker => {
-            PathBuf::from("/var/lib/serviceradar").join(component_type.as_str())
-        }
-        DeploymentType::Kubernetes => {
-            PathBuf::from("/var/lib/serviceradar").join(component_type.as_str())
-        }
-        DeploymentType::BareMetal => {
-            PathBuf::from("/var/lib/serviceradar").join(component_type.as_str())
-        }
-    }
+fn get_storage_path(_deployment_type: &DeploymentType, component_type: ComponentType) -> PathBuf {
+    PathBuf::from("/var/lib/serviceradar").join(component_type.as_str())
 }
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly points out that the match statement is redundant as all arms return the same value. Simplifying it improves conciseness. The impact is minor, but it's a valid code quality improvement.

Low
  • Update
Imported GitHub PR comment. Original author: @qodo-code-review[bot] Original URL: https://github.com/carverauto/serviceradar/pull/2059#issuecomment-3613893651 Original created: 2025-12-04T19:00:21Z --- ## PR Code Suggestions ✨ <!-- 2da1cfb --> Explore these optional code suggestions: <table><thead><tr><td><strong>Category</strong></td><td align=left><strong>Suggestion&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </strong></td><td align=center><strong>Impact</strong></td></tr><tbody><tr><td rowspan=1>Security</td> <td> <details><summary>Default to HTTPS for better security</summary> ___ **In the <code>ensure_scheme</code> function, change the default protocol to <code>https://</code> instead <br>of <code>http://</code> for better security. An exception can be made to use <code>http://</code> for <br>localhost addresses.** [rust/edge-onboarding/src/download.rs [133-143]](https://github.com/carverauto/serviceradar/pull/2059/files#diff-9e5dc37d31894894a78d7eb118c72f7303a8be99adb992ac51de9e664fdee2f2R133-R143) ```diff /// Ensure the URL has a scheme (http:// or https://). fn ensure_scheme(host: &str) -> Result<String> { let host = host.trim(); if host.is_empty() { return Err(Error::CoreApiHostRequired); } if host.starts_with("http://") || host.starts_with("https://") { return Ok(host.to_string()); } - Ok(format!("http://{}", host)) + // Default to https for security, but allow http for localhost. + if host.starts_with("localhost") || host.starts_with("127.0.0.1") { + Ok(format!("http://{}", host)) + } else { + Ok(format!("https://{}", host)) + } } ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=0 --> <details><summary>Suggestion importance[1-10]: 8</summary> __ Why: This suggestion correctly identifies a security weakness where the client might communicate over unencrypted HTTP by default. Changing the default to HTTPS is a significant security improvement that aligns with secure-by-default principles. </details></details></td><td align=center>Medium </td></tr><tr><td rowspan=2>General</td> <td> <details><summary>Improve legacy token parsing logic</summary> ___ **In <code>parse_legacy_token</code>, replace <code>find</code> with <code>rfind</code> when searching for separators. <br>This makes parsing more robust by splitting on the last occurrence of a <br>separator, which is a common pattern for <code>id:token</code> formats.** [rust/edge-onboarding/src/token.rs [96-133]](https://github.com/carverauto/serviceradar/pull/2059/files#diff-522ddb82ad9919bd416394e34bbfd4ce3e8793723b260a08843d7500ae0b98cdR96-R133) ```diff fn parse_legacy_token( raw: &str, fallback_package_id: Option<&str>, fallback_core_url: Option<&str>, ) -> Result<TokenPayload> { let mut payload = TokenPayload { package_id: fallback_package_id.unwrap_or_default().to_string(), download_token: String::new(), core_url: fallback_core_url.map(String::from), }; let mut legacy = raw.to_string(); // Check for URL@remainder format if let Some(at_idx) = legacy.find('@') { let maybe_url = legacy[..at_idx].trim(); let remainder = legacy[at_idx + 1..].trim(); if looks_like_url(maybe_url) && !remainder.is_empty() { payload.core_url = Some(maybe_url.to_string()); legacy = remainder.to_string(); } } - // Try to split by common separators + // Try to split by common separators, using the last occurrence for sep in [':', '/', '|', ','] { - if let Some(idx) = legacy.find(sep) { + if let Some(idx) = legacy.rfind(sep) { payload.package_id = legacy[..idx].trim().to_string(); payload.download_token = legacy[idx + 1..].trim().to_string(); validate_token_payload(&payload)?; return Ok(payload); } } // No separator found - treat entire string as download token payload.download_token = legacy.trim().to_string(); validate_token_payload(&payload)?; Ok(payload) } ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=1 --> <details><summary>Suggestion importance[1-10]: 7</summary> __ Why: The suggestion correctly identifies a potential ambiguity in parsing legacy tokens and proposes using `rfind` to make the logic more robust. This is a good improvement for handling edge cases where separators might appear in the package ID. </details></details></td><td align=center>Medium </td></tr><tr><td> <details><summary>Simplify function with redundant match arms</summary> ___ **Simplify the <code>get_storage_path</code> function by removing the redundant <code>match</code> <br>statement, as all deployment types currently resolve to the same storage path. <br>The first argument can be marked as unused.** [rust/edge-onboarding/src/lib.rs [313-326]](https://github.com/carverauto/serviceradar/pull/2059/files#diff-5d04060c8999c31f6f944993ba5c30a1d7cc1882441105b20e6aae0aa0c2eb52R313-R326) ```diff /// Get storage path based on deployment type. -fn get_storage_path(deployment_type: &DeploymentType, component_type: ComponentType) -> PathBuf { - match deployment_type { - DeploymentType::Docker => { - PathBuf::from("/var/lib/serviceradar").join(component_type.as_str()) - } - DeploymentType::Kubernetes => { - PathBuf::from("/var/lib/serviceradar").join(component_type.as_str()) - } - DeploymentType::BareMetal => { - PathBuf::from("/var/lib/serviceradar").join(component_type.as_str()) - } - } +fn get_storage_path(_deployment_type: &DeploymentType, component_type: ComponentType) -> PathBuf { + PathBuf::from("/var/lib/serviceradar").join(component_type.as_str()) } ``` - [ ] **Apply / Chat** <!-- /improve --apply_suggestion=2 --> <details><summary>Suggestion importance[1-10]: 4</summary> __ Why: The suggestion correctly points out that the `match` statement is redundant as all arms return the same value. Simplifying it improves conciseness. The impact is minor, but it's a valid code quality improvement. </details></details></td><td align=center>Low </td></tr> <tr><td align="center" colspan="2"> - [ ] Update <!-- /improve_multi --more_suggestions=true --> </td><td></td></tr></tbody></table>
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
1week

   
2weeks

   
Failed compliance check

   
IP cameras

   
NATS

NATS JetStream

  
Possible security concern

   
Review effort 1/5

   
Review effort 2/5

   
Review effort 3/5

   
Review effort 4/5

   
Review effort 5/5

   
UI

   
aardvark

   
accessibility

   
amd64

   
api

   
arm64

   
auth

   
back-end

   
bgp

   
blog

   
bug

Something isn't working

  
build

   
checkers

   
ci-cd

continuous integration-continuous deployments

  
cleanup

   
cnpg

cloud-native postgres

  
codex

   
core

core service

  
dependencies

Pull requests that update a dependency file

  
device-management

   
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
dusk

   
ebpf

   
enhancement

New feature or request

  
eta 1d

   
eta 1hr

   
eta 3d

   
eta 3hr

   
feature

   
fieldsurvey

   
github_actions

Pull requests that update GitHub Actions code

  
go

Pull requests that update Go code

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
javascript

Pull requests that update Javascript code

  
k8s

   
log-collector

   
mapper

   
mtr

multi traceroute

  
needs-triage

   
netflow

   
network-sweep

   
observability

   
oracle

Oracle Linux related issues

  
otel

opentelemetry logs, traces, metrics

  
plug-in

   
proton

timeplus proton streaming database

  
python

   
question

Further information is requested

  
reddit

   
redhat

   
research

   
rperf

   
rperf-checker

   
rust

Pull requests that update rust code

  
sdk

   
security

   
serviceradar-agent

   
serviceradar-agent-gateway

   
serviceradar-web

   
serviceradar-web-ng

   
siem

   
snmp

   
sysmon

   
topology

   
ubiquiti

   
wasm

   
wontfix

This will not be worked on

  
zen-engine
No labels
1week
2weeks
Failed compliance check
IP cameras
NATS
Possible security concern
Review effort 1/5
Review effort 2/5
Review effort 3/5
Review effort 4/5
Review effort 5/5
UI
aardvark
accessibility
amd64
api
arm64
auth
back-end
bgp
blog
bug
build
checkers
ci-cd
cleanup
cnpg
codex
core
dependencies
device-management
documentation
duplicate
dusk
ebpf
enhancement
eta 1d
eta 1hr
eta 3d
eta 3hr
feature
fieldsurvey
github_actions
go
good first issue
help wanted
invalid
javascript
k8s
log-collector
mapper
mtr
needs-triage
netflow
network-sweep
observability
oracle
otel
plug-in
proton
python
question
reddit
redhat
research
rperf
rperf-checker
rust
sdk
security
serviceradar-agent
serviceradar-agent-gateway
serviceradar-web
serviceradar-web-ng
siem
snmp
sysmon
topology
ubiquiti
wasm
wontfix
zen-engine
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!2507
No description provided.