Skip to content

Remove unused export from SslBumpConfig in security-critical module#2438

Merged
lpcox merged 4 commits intomainfrom
copilot/remove-unused-export-ssl-bump-config
May 3, 2026
Merged

Remove unused export from SslBumpConfig in security-critical module#2438
lpcox merged 4 commits intomainfrom
copilot/remove-unused-export-ssl-bump-config

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented May 3, 2026

SslBumpConfig was exported from src/ssl-bump.ts despite never being imported anywhere — it's only used as the parameter type for the file-internal generateSessionCa function. Unnecessary exports in a security-critical module (SSL CA generation for TLS interception) widen the API surface and invite misuse by callers bypassing intended validation.

Change

  • src/ssl-bump.ts: Drop export from SslBumpConfig interface, making it module-private.
// before
export interface SslBumpConfig {  }

// after
interface SslBumpConfig {  }

Copilot AI changed the title [WIP] Remove unused exported interface SslBumpConfig from ssl-bump.ts Remove unused export from SslBumpConfig in security-critical module May 3, 2026
Copilot finished work on behalf of lpcox May 3, 2026 13:59
Copilot AI requested a review from lpcox May 3, 2026 13:59
@lpcox lpcox marked this pull request as ready for review May 3, 2026 22:59
Copilot AI review requested due to automatic review settings May 3, 2026 22:59
@lpcox lpcox requested a review from Mossaka as a code owner May 3, 2026 22:59
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

✅ Coverage Check Passed

Overall Coverage

Metric Base PR Delta
Lines 85.76% 85.77% ➡️ +0.01%
Statements 85.64% 85.71% 📈 +0.07%
Functions 88.11% 88.15% 📈 +0.04%
Branches 78.65% 78.72% 📈 +0.07%
📁 Per-file Coverage Changes (2 files)
File Lines (Before → After) Statements (Before → After)
src/pid-tracker.ts 97.3% → 97.2% (-0.15%) 95.8% → 95.6% (-0.22%)
src/docker-manager.ts 87.4% → 87.6% (+0.16%) 87.0% → 87.4% (+0.36%)

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

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

Reduces the exported API surface of the TLS interception (SSL bump) CA generation module by making the SslBumpConfig type module-private.

Changes:

  • Remove export from the SslBumpConfig interface in src/ssl-bump.ts.
Show a summary per file
File Description
src/ssl-bump.ts Makes SslBumpConfig non-exported to narrow the module’s public surface area.

Copilot's findings

Tip

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

Comments suppressed due to low confidence (1)

src/ssl-bump.ts:39

  • PR description says SslBumpConfig is only used for a file-internal generateSessionCa, but generateSessionCa is currently exported from this module. If the intent is to keep generateSessionCa internal to reduce API surface, it should be made non-exported (or moved behind a narrower exported wrapper) instead of only un-exporting the config interface.
interface SslBumpConfig {
  • Files reviewed: 1/1 changed files
  • Comments generated: 1

Comment thread src/ssl-bump.ts Outdated
* Configuration for SSL Bump CA generation
*/
export interface SslBumpConfig {
interface SslBumpConfig {
@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.

@github-actions

This comment has been minimized.

@lpcox
Copy link
Copy Markdown
Collaborator

lpcox commented May 3, 2026

@copilot address the review feedback

… non-exported interface

Agent-Logs-Url: https://github.com/github/gh-aw-firewall/sessions/d3d6437d-fcec-4fe2-b8a8-fd6939900785

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 3, 2026

@copilot address the review feedback

Fixed in 8258b96. Since declaration: true is set in tsconfig.json, an exported function can't reference a non-exported named type in its signature. Rather than re-exporting SslBumpConfig, I inlined the object type directly in generateSessionCa's parameter:

export async function generateSessionCa(config: { workDir: string; commonName?: string; validityDays?: number }): Promise<CaFiles>

SslBumpConfig stays private (no export), the build compiles cleanly, and the public API surface is unchanged.

Copilot finished work on behalf of lpcox May 3, 2026 23:15
@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 3, 2026

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

Copy link
Copy Markdown
Contributor Author

Copilot AI commented May 3, 2026

@copilot fix https://github.com/github/gh-aw-firewall/actions/runs/25293627801/job/74149316879?pr=2438

Fixed in a30c75c. The ESLint no-unused-vars error was caused by the SslBumpConfig interface still being declared after its only usage (the function parameter) was replaced with an inline type. The fix was to remove the interface entirely — it's no longer needed anywhere.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Smoke Test: Copilot BYOK — PASS ✅

Test Result
GitHub MCP (merged PR: "fix: unexport internal DlpPattern...")
GitHub.com connectivity
File write/read (smoke-test-copilot-byok-25293919465.txt)
BYOK inference (agent → api-proxy → api.githubcopilot.com)

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

PR by @Copilot · assignees: @lpcox, @Copilot

🔑 BYOK report filed by Smoke Copilot BYOK

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Smoke Test Results

✅ GitHub MCP: #2437, #2436
✅ Playwright: github.com title verified
✅ File Write: smoke-test file created
✅ Bash: file verified

Status: PASS

💥 [THE END] — Illustrated by Smoke Claude

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

🤖 Smoke Test Results

Test Status
GitHub MCP (list_pull_requests)
GitHub.com connectivity (HTTP 200)
File write/read ❌ (template vars unresolved)

PR: "Remove unused export from SslBumpConfig in security-critical module"
Author: @Copilot | Assignees: @lpcox, @Copilot

Overall: FAIL — file test could not be verified due to unexpanded workflow template variables.

📰 BREAKING: Report filed by Smoke Copilot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Chroot Smoke Test Results

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

Result: ❌ Not all tests passed — Python and Node.js versions differ between host and chroot.

Tested by Smoke Chroot

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

PRs: "fix: unexport internal DlpPattern interface in src/dlp.ts"; "refactor(cli-workflow): unexport internal-only interfaces"
GitHub MCP review: ❌
Safe Inputs GH CLI: ❌
Playwright title check: ✅
Tavily search: ❌
File write: ✅
Bash cat verify: ✅
Discussion comment: ✅
Build AWF: ✅
Overall: 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

github-actions Bot commented May 3, 2026

Smoke Test: Gemini Engine Validation

  • GitHub MCP Testing: ❌ (Tool not accessible via standard calls)
  • GitHub.com Connectivity: ✅ (HTTP 200)
  • File Writing Testing: ✅ (Verified)
  • Bash Tool Testing: ✅ (Verified)

Overall status: FAIL

💎 Faceted by Smoke Gemini

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

🏗️ 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

Note: Java required using -Dmaven.repo.local=/tmp/gh-aw/agent/m2repo as the default /home/runner/.m2/repository path was not writable (permission denied).

Generated by Build Test Suite for issue #2438 · ● 352.5K ·

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Smoke Test: Services Connectivity

Check Result
Redis PING ❌ timeout/no response
PostgreSQL pg_isready ❌ no response
PostgreSQL SELECT 1 ❌ skipped (pg unreachable)

Overall: FAILhost.docker.internal services (Redis :6379, PostgreSQL :5432) are not reachable from this environment.

🔌 Service connectivity validated by Smoke Services

@lpcox lpcox merged commit a791df8 into main May 3, 2026
65 of 68 checks passed
@lpcox lpcox deleted the copilot/remove-unused-export-ssl-bump-config branch May 3, 2026 23:53
@github-actions github-actions Bot mentioned this pull request May 4, 2026
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] Unused exported interface in security-critical module src/ssl-bump.ts: SslBumpConfig

3 participants