Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,8 @@
"@opentelemetry/otlp-proto-exporter-base": "^0.51.1",
"@azure/openai": "^2.0.0",
"node-forge": ">=1.4.0",
"picomatch": ">=2.3.2",
"picomatch": ">=4.0.4",
"path-to-regexp": ">=8.4.0",
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Apr 1, 2026

Choose a reason for hiding this comment

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

P1: The new path-to-regexp override conflicts with the existing resolutions pin (8.3.0), so this version bump may not actually take effect.

Prompt for AI agents
Check if this issue is valid β€” if so, understand the root cause and fix it. At package.json, line 429:

<comment>The new `path-to-regexp` override conflicts with the existing `resolutions` pin (`8.3.0`), so this version bump may not actually take effect.</comment>

<file context>
@@ -425,7 +425,8 @@
       "node-forge": ">=1.4.0",
-      "picomatch": ">=2.3.2",
+      "picomatch": ">=4.0.4",
+      "path-to-regexp": ">=8.4.0",
       "express": ">=4.19.2",
       "js-yaml": ">=4.1.1",
</file context>
Fix with Cubic

"express": ">=4.19.2",
"js-yaml": ">=4.1.1",
"yaml": ">=2.8.3",
Expand Down
38 changes: 16 additions & 22 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions security/tests/test_compliance_monitor.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import sys
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

sys is imported but not used in the test module. Removing unused imports keeps the test file minimal and avoids lint failures if the repo enforces them.

Suggested change
import sys

Copilot uses AI. Check for mistakes.
import os
import importlib.util

current_dir = os.path.dirname(os.path.abspath(__file__))
parent_dir = os.path.dirname(current_dir)

spec = importlib.util.spec_from_file_location('compliance_monitor', os.path.join(parent_dir, 'compliance-monitor.py'))
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

spec_from_file_location(...) and spec.loader can be None; calling importlib.util.module_from_spec(spec) or spec.loader.exec_module(...) would then raise an AttributeError/TypeError with a confusing message. Add an explicit assertion/guard that spec and spec.loader are not None so failures are clearer and deterministic.

Suggested change
spec = importlib.util.spec_from_file_location('compliance_monitor', os.path.join(parent_dir, 'compliance-monitor.py'))
module_path = os.path.join(parent_dir, 'compliance-monitor.py')
spec = importlib.util.spec_from_file_location('compliance_monitor', module_path)
if spec is None or spec.loader is None:
raise ImportError(f"Cannot load module 'compliance_monitor' from {module_path!r}: invalid spec or loader")

Copilot uses AI. Check for mistakes.
compliance_monitor = importlib.util.module_from_spec(spec)
spec.loader.exec_module(compliance_monitor)
Comment on lines +5 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The dynamic import is brittle as written: spec or spec.loader can be None (bad path, packaging changes), which will fail with a less clear error at import time. This should fail fast with a clear assertion, and it’s better to avoid module-level side effects by deferring the load into a fixture.

Suggestion

Add explicit checks for spec/spec.loader and consider moving the import into a pytest fixture to avoid import-time side effects.

from pathlib import Path
import importlib.util
import pytest

@pytest.fixture(scope="module")
def compliance_monitor_module():
    path = Path(__file__).resolve().parents[1] / "compliance-monitor.py"
    spec = importlib.util.spec_from_file_location("compliance_monitor", path)
    assert spec is not None and spec.loader is not None
    module = importlib.util.module_from_spec(spec)
    spec.loader.exec_module(module)
    return module

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Comment on lines +1 to +10
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The test uses a dynamic file-based import and directly executes compliance-monitor.py. This is brittle (path-dependent) and bypasses normal package imports, making refactors harder and potentially causing subtle import/side-effect issues when the module is executed during collection.

If the hyphenated filename is the reason for this approach, consider adding an importable wrapper module (e.g., security/compliance_monitor.py) that imports/loads the implementation, then import that wrapper normally in tests.

Suggestion

Prefer a normal import path for tests by introducing a wrapper module and importing it directly.

Example:

# security/compliance_monitor.py (new)
import importlib.util
from pathlib import Path

_impl = Path(__file__).with_name("compliance-monitor.py")
spec = importlib.util.spec_from_file_location("security.compliance_monitor_impl", _impl)
assert spec and spec.loader
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)

ComplianceMonitor = mod.ComplianceMonitor

Then in the test:

from security.compliance_monitor import ComplianceMonitor

def test_check_data_encryption():
    assert ComplianceMonitor().check_data_encryption() is True

Reply with "@CharlieHelps yes please" if you’d like me to add a commit with this refactor.


def test_check_data_encryption():
monitor = compliance_monitor.ComplianceMonitor()
result = monitor.check_data_encryption()
assert result is True
Comment on lines +12 to +15
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

As a "unit" test, this currently calls the real check_data_encryption() implementation with no isolation. If that method depends on the host environment (filesystem, OS settings, external commands, cloud metadata), this will be flaky and may pass/fail depending on where CI runs.

A better unit test would stub/mock the dependency boundary that check_data_encryption() uses and assert behavior deterministically (including at least one negative path).

Suggestion

Refactor the test to patch the internal dependency that drives the encryption decision (e.g., a helper method, config reader, or command runner used by check_data_encryption) so the test is deterministic, and add a failing/False-path assertion.

Example pattern:

def test_check_data_encryption_true(monkeypatch, compliance_monitor_module):
    monitor = compliance_monitor_module.ComplianceMonitor()
    monkeypatch.setattr(monitor, "_encryption_enabled", lambda: True)
    assert monitor.check_data_encryption() is True

def test_check_data_encryption_false(monkeypatch, compliance_monitor_module):
    monitor = compliance_monitor_module.ComplianceMonitor()
    monkeypatch.setattr(monitor, "_encryption_enabled", lambda: False)
    assert monitor.check_data_encryption() is False

(Replace _encryption_enabled with the actual dependency seam used in the implementation.)

Reply with "@CharlieHelps yes please" if you'd like me to add a commit with this suggestion.

Comment on lines +12 to +15
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

The PR description focuses on adding a single unit test, but this PR also changes multiple TypeScript files (introducing any casts) and modifies package.json dependency constraints. Either update the PR description to reflect these additional changes or split the unrelated changes into separate PRs to keep review scope aligned.

Copilot uses AI. Check for mistakes.
2 changes: 1 addition & 1 deletion src/components/admin/dlp/DLPRuleEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ export default function DLPRuleEditor() {
if (!(event instanceof CustomEvent)) {
return
}
const detail = event.detail as EditorRule
const detail = event.detail as any as EditorRule
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Since you already guard event instanceof CustomEvent, you can avoid any by typing the event as CustomEvent<EditorRule> (or by narrowing with a typed helper) so event.detail is strongly typed. This prevents any from spreading into setCurrentRule(...) and makes the editor event contract explicit.

Copilot uses AI. Check for mistakes.
setCurrentRule({ ...defaultRule, ...detail, matchPattern: detail.matchPattern ?? '' })
setIsEditing(true)
}
Expand Down
2 changes: 1 addition & 1 deletion src/components/ui/tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ export function TabsList({ children, className = '' }: TabsListProps) {
const tabs = Array.from(
list.querySelectorAll('[role="tab"]:not([disabled])'),
)
const index = tabs.indexOf(document.activeElement as HTMLElement)
const index = tabs.indexOf(document.activeElement as any as HTMLElement)
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

document.activeElement can be null or a non-HTMLElement. Casting through any hides that and makes future refactors riskier. Prefer using a small runtime check (e.g., store activeElement, verify it’s an HTMLElement, then compute index) to avoid the any cast.

Suggested change
const index = tabs.indexOf(document.activeElement as any as HTMLElement)
const activeElement = document.activeElement
if (!(activeElement instanceof HTMLElement)) return
const index = tabs.indexOf(activeElement)

Copilot uses AI. Check for mistakes.

if (index < 0) return

Expand Down
18 changes: 9 additions & 9 deletions src/lib/logging/standardized-logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,43 +19,43 @@ export type Logger = {

// Factory functions for named loggers - delegate to canonical getLogger so tests can mock
export function getBiasDetectionLogger(scope: string): Logger {
return getLogger(`bias-detection:${scope}`) as unknown as Logger
return getLogger(`bias-detection:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getClinicalAnalysisLogger(scope: string): Logger {
return getLogger(`clinical-analysis:${scope}`) as unknown as Logger
return getLogger(`clinical-analysis:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getAiServiceLogger(scope: string): Logger {
return getLogger(`ai-service:${scope}`) as unknown as Logger
return getLogger(`ai-service:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getApiEndpointLogger(scope: string): Logger {
return getLogger(`api-endpoint:${scope}`) as unknown as Logger
return getLogger(`api-endpoint:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getComponentLogger(scope: string): Logger {
return getLogger(`component:${scope}`) as unknown as Logger
return getLogger(`component:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getServiceLogger(scope: string): Logger {
return getLogger(`service:${scope}`) as unknown as Logger
return getLogger(`service:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getSecurityLogger(scope: string): Logger {
return getLogger(`security:${scope}`) as unknown as Logger
return getLogger(`security:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getAdvancedPHILogger(
config: { enableLogCollection?: boolean } = {},
): Logger {
return getLogger(
`advanced-phi${config.enableLogCollection ? ':collect' : ''}`,
) as unknown as Logger
) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

export function getHipaaCompliantLogger(scope: string): Logger {
return getLogger(`hipaa:${scope}`) as unknown as Logger
return getLogger(`hipaa:${scope}`) as any as Logger
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

Replacing as unknown as Logger with as any as Logger weakens type safety (it bypasses checking rather than enforcing a safe narrowing). Prefer keeping unknown casts, or better: align the types so getLogger(...) returns the exported Logger interface (or introduce a typed adapter/wrapper) and avoid double-casting altogether.

Copilot uses AI. Check for mistakes.
}

// Default/general loggers - provide thin runtime proxies to getLogger
Expand Down
2 changes: 1 addition & 1 deletion src/lib/utils/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class Logger {
return obj
}

const newObj = { ...(obj as Record<string, unknown>) }
const newObj = { ...(obj as any as Record<string, unknown>) }
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

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

This introduces an any cast in a place that looks like it should already be guarded by runtime checks. Prefer removing any and adjusting the preceding type guards / narrowing so the spread works with a safer type (e.g., Record<string, unknown> after confirming obj is a plain object). This keeps the redaction path type-safe and avoids masking incorrect inputs.

Copilot uses AI. Check for mistakes.
for (const key of keys) {
if (key in newObj) {
newObj[key] = '[REDACTED]'
Expand Down
Loading