Skip to content

Make DLP_PATTERNS internal and decouple tests from module internals#3398

Merged
lpcox merged 3 commits into
mainfrom
copilot/remove-dead-export-dlp-patterns
May 19, 2026
Merged

Make DLP_PATTERNS internal and decouple tests from module internals#3398
lpcox merged 3 commits into
mainfrom
copilot/remove-dead-export-dlp-patterns

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 19, 2026

src/dlp.ts exposed DLP_PATTERNS as a public export even though it is only used internally by generateDlpSquidConfig(). This created unnecessary API surface in a security-critical module and test coupling to implementation details.

  • API surface reduction

    • Removed export from DLP_PATTERNS in src/dlp.ts.
    • Kept pattern usage internal to config generation logic.
  • Test strategy update (behavior over internals)

    • Updated src/dlp.test.ts to stop importing DLP_PATTERNS.
    • Added test helpers that derive regex patterns from generateDlpSquidConfig().aclLines.
    • Preserved detection coverage by asserting expected generated regex matches for representative credential inputs.
  • Scope of change

    • No functional change to DLP matching or generated Squid deny behavior; this is an interface cleanup plus test refactor.
// before
export const DLP_PATTERNS: DlpPattern[] = [ ... ];

// after
const DLP_PATTERNS: DlpPattern[] = [ ... ];

Copilot AI changed the title [WIP] Remove dead export DLP_PATTERNS in src/dlp.ts Make DLP_PATTERNS internal and decouple tests from module internals May 19, 2026
Copilot finished work on behalf of lpcox May 19, 2026 05:20
Copilot AI requested a review from lpcox May 19, 2026 05:20
@lpcox lpcox marked this pull request as ready for review May 19, 2026 14:21
Copilot AI review requested due to automatic review settings May 19, 2026 14:21
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 19, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 95.40% 95.47% 📈 +0.07%
Statements 95.22% 95.29% 📈 +0.07%
Functions 96.55% 96.55% ➡️ +0.00%
Branches 87.87% 87.91% 📈 +0.04%
📁 Per-file Coverage Changes (1 files)
File Lines (Before → After) Statements (Before → After)
src/config-writer.ts 83.0% → 85.6% (+2.54%) 83.0% → 85.6% (+2.54%)

Coverage comparison generated by scripts/ci/compare-coverage.ts

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces public API surface in the security-sensitive DLP module by making DLP_PATTERNS internal-only, and updates unit tests to validate behavior via generateDlpSquidConfig() output rather than importing internal module constants.

Changes:

  • Removed the export from DLP_PATTERNS in src/dlp.ts to avoid exposing internal pattern definitions.
  • Refactored src/dlp.test.ts to derive regexes from generateDlpSquidConfig().aclLines and assert representative credential-detection behavior.
Show a summary per file
File Description
src/dlp.ts Makes DLP_PATTERNS internal to reduce exported API surface.
src/dlp.test.ts Updates tests to validate generated ACL patterns and matching behavior without importing DLP_PATTERNS.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 2/2 changed files
  • Comments generated: 3

Comment thread src/dlp.ts
* - Avoid overly broad patterns that would block legitimate traffic
*/
export const DLP_PATTERNS: DlpPattern[] = [
const DLP_PATTERNS: DlpPattern[] = [
Comment thread src/dlp.test.ts Outdated
Comment on lines +3 to +9
const DLP_ACL_PREFIX = 'acl dlp_blocked url_regex -i ';

function getDlpRegexPatterns(): string[] {
const { aclLines } = generateDlpSquidConfig();
return aclLines
.filter(line => line.startsWith(DLP_ACL_PREFIX))
.map(line => line.slice(DLP_ACL_PREFIX.length));
Comment thread src/dlp.test.ts Outdated
Comment on lines 12 to 14
function scanForCredentialsUsingPatterns(input: string): string[] {
return DLP_PATTERNS
.filter(pattern => new RegExp(pattern.regex, 'i').test(input))
.map(pattern => pattern.name);
return getDlpRegexPatterns().filter(regex => new RegExp(regex, 'i').test(input));
}
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 19, 2026

@copilot address review feedback

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 19, 2026

@copilot address review feedback

Addressed in ac7ca05.

  • Updated ACL extraction to use a regex capture (/^acl\s+dlp_blocked\s+url_regex\s+-i\s+(.+)$/) so tests are less brittle to spacing changes.
  • Renamed the helper to findMatchingDlpRegexes and updated local variable names to reflect that it returns matching regex patterns.
  • Kept DLP_PATTERNS internal as intended for API-surface reduction; this export removal is a deliberate API change and should be handled in release/version notes.

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

✅ GitHub API: 2 PR entries found
✅ Playwright: GitHub page title verified
✅ File verify: smoke-test-claude-26104352024.txt confirmed

PASS — All smoke tests passed.

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

🔥 Smoke Test: Copilot BYOK (Offline) Mode

Test Result
GitHub MCP (list PRs) ✅ PR #3396 returned
GitHub.com connectivity ⚠️ Pre-step vars not expanded
File write/read ⚠️ Pre-step vars not expanded
BYOK inference (agent → api-proxy → api.githubcopilot.com) ✅ Responding now

Running in BYOK offline mode (COPILOT_OFFLINE=true) via api-proxy → api.githubcopilot.com.

Author: @Copilot | Assignees: @lpcox @Copilot

Overall: PASS (core BYOK path verified ✅)

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

🔬 Smoke Test Results

Test Status
GitHub MCP connectivity ✅ PR listed successfully
GitHub.com HTTP ⚠️ Pre-step data unavailable (template not expanded)
File write/read ⚠️ Pre-step data unavailable (template not expanded)

PR: Make DLP_PATTERNS internal and decouple tests from module internals
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: PARTIAL — MCP ✅, pre-step smoke data was not injected (template variables unexpanded).

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke test: FAIL
PRs reviewed: Internalize DOMAIN_CHAR_PATTERN and decouple tests from non-API regex internals | Privatize ruleset internals and test rules parsing through public API
GitHub PR review: ✅ | safeinputs-gh: ❌ | Playwright: ✅ | Tavily: ❌
File write/read: ✅ | Discussion: ✅ | Build: ✅
Overall status: FAIL

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • registry.npmjs.org

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "registry.npmjs.org"

See Network Configuration for more information.

🔮 The oracle has spoken through Smoke Codex

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

  • GitHub MCP: ❌ (mcpscripts not found)
  • GitHub Connectivity: ❌ (Status 000)
  • File Writing: ✅
  • Bash Tool: ✅

Overall Status: FAIL

PR titles found: #3385 (only one in shallow history)

Warning

Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

  • localhost

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "localhost"

See Network Configuration for more information.

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

🏗️ Build Test Suite Results

Ecosystem Project Build/Install Tests Status
Bun elysia 1/1 passed ✅ PASS
Bun hono 1/1 passed ✅ PASS
C++ fmt N/A ✅ PASS
C++ json N/A ✅ PASS
Deno oak N/A 1/1 passed ✅ PASS
Deno std N/A 1/1 passed ✅ PASS
.NET hello-world N/A ✅ PASS
.NET json-parse N/A ✅ PASS
Go color 1/1 passed ✅ PASS
Go env 1/1 passed ✅ PASS
Go uuid 1/1 passed ✅ PASS
Java gson 1/1 passed ✅ PASS
Java caffeine 1/1 passed ✅ PASS
Node.js clsx All passed ✅ PASS
Node.js execa All passed ✅ PASS
Node.js p-limit All passed ✅ PASS
Rust fd 1/1 passed ✅ PASS
Rust zoxide 1/1 passed ✅ PASS

Overall: 8/8 ecosystems passed — ✅ PASS

Generated by Build Test Suite for issue #3398 · ● 3.5M ·

@github-actions
Copy link
Copy Markdown
Contributor

Chroot Smoke Test Results

Runtime Host Version Chroot Version Match?
Python Python 3.12.13 Python 3.12.3
Node.js v24.15.0 v20.20.2
Go go1.22.12 go1.22.12

Result: FAILED — Python and Node.js versions differ between host and chroot environment.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

Smoke Test Results

  • ❌ Redis (host.docker.internal:6379): timeout — no response
  • ❌ PostgreSQL pg_isready (host.docker.internal:5432): no response
  • ❌ PostgreSQL SELECT 1: unreachable

host.docker.internal resolves to 172.17.0.1 but both ports timed out. Service containers may not be running or are not accessible from this environment.

Overall: FAIL

🔌 Service connectivity validated by Smoke Services

Copilot finished work on behalf of lpcox May 19, 2026 15:00
@lpcox lpcox merged commit f72ce5c into main May 19, 2026
65 of 68 checks passed
@lpcox lpcox deleted the copilot/remove-dead-export-dlp-patterns branch May 19, 2026 15:10
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.

[Export Audit] Dead export DLP_PATTERNS in security-critical module src/dlp.ts (recurring — prior #2631)

3 participants