Skip to content

fix(core): harden config, path, and output trust boundaries#976

Merged
ben-ranford merged 6 commits into
mainfrom
bug/issues-839-865-866-867-868-869-870-security-hardening
Jun 10, 2026
Merged

fix(core): harden config, path, and output trust boundaries#976
ben-ranford merged 6 commits into
mainfrom
bug/issues-839-865-866-867-868-869-870-security-hardening

Conversation

@ben-ranford

@ben-ranford ben-ranford commented Jun 2, 2026

Copy link
Copy Markdown
Owner

Summary

Security hardening bundle for #839, #865, #866, #867, #868, #869, and #870.

Changes

  • remove trust escalation from explicit --config handling and reject remote policy packs without fetching
  • anchor runtime hook lookup to fixed install-layout roots instead of executable-ancestor walking
  • decode quoted git paths, escape Slack fallback text, expand webhook redaction, sanitize report table output, and reject broken symlink cache escapes
  • add focused regression coverage across git/workspace, runtime, notify, report, thresholds, safeio, analysis, and app packages

Validation

Commands run locally by the worker:

go test ./internal/gitexec ./internal/thresholds ./internal/runtime ./internal/workspace ./internal/notify ./internal/report ./internal/analysis ./internal/safeio ./internal/app
go test ./internal/thresholds ./internal/runtime

Follow-up validation will rerun after the lint fix is pushed.

Risk and compatibility

  • Breaking changes: remote policy packs are now rejected consistently; explicit configs outside the analyzed repo can still load local packs from their own directory.
  • Migration required: none expected for trusted local config usage.
  • Performance impact: none expected.
  • Memory benchmark impact: none expected.

Closes #839
Closes #865
Closes #866
Closes #867
Closes #868
Closes #869
Closes #870

Checklist

  • Tests added/updated for behavior changes
  • Docs updated (README/docs/schema) if needed
  • memory-approved requested/applied if intentional memory benchmark regressions exceed CI thresholds
  • No unrelated changes included
  • Ready for review

Copilot AI review requested due to automatic review settings June 2, 2026 13:16
@ben-ranford ben-ranford added this to the v1.6.0 milestone Jun 2, 2026
@ben-ranford ben-ranford added the bug Something isn't working label Jun 2, 2026
@github-actions

github-actions Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Memory Benchmarks

Thresholds: bytes/op <= +15.0%, allocs/op <= +10.0%

Benchmark Base B/op Head B/op Delta B/op Base allocs/op Head allocs/op Delta allocs/op Status
github.com/ben-ranford/lopper/internal/lang/shared/BenchmarkCountUsage 25632.3 25632.0 -0.0% 375.0 375.0 +0.0% ok
github.com/ben-ranford/lopper/internal/lang/shared/BenchmarkCountUsageRegexPerIdentifier 416775.3 414127.3 -0.6% 3067.7 3067.0 -0.0% ok
github.com/ben-ranford/lopper/internal/report/BenchmarkFormatLargeTable 254737.3 256167.0 +0.6% 3014.0 3064.0 +1.7% ok

Result: memory benchmark gate passed.

Approval: not required.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Security-hardening sweep that tightens multiple trust boundaries across config/policy-pack loading, runtime hook discovery, git path parsing, notification redaction/escaping, CLI table output sanitization, and cache path safety.

Changes:

  • Disable remote policy packs (no fetching) and adjust policy-pack trust handling/error messaging + tests.
  • Anchor runtime hook lookup to fixed, non-ancestor-walk roots and add targeted tests.
  • Normalize/sanitize repo-controlled strings at output boundaries (git quoted paths decoding, Slack fallback escaping, webhook redaction expansion, table-output terminal control scrubbing, cache symlink escape rejection).

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
internal/workspace/workspace_changed_files_test.go Adds regression coverage for decoding git-quoted non-ASCII paths from diff/status.
internal/workspace/changed_files.go Decodes git C-quoted paths (defense-in-depth) in diff/status parsing.
internal/thresholds/config.go Updates policy resolver trust initialization signature for hardened pack resolution.
internal/thresholds/config_test.go Updates/reshapes tests to assert remote policy packs are rejected without fetching.
internal/thresholds/config_packs.go Removes “explicit config implies remote trust” and hard-disables remote packs; adjusts local-root trust.
internal/thresholds/config_cov_more_http_test.go Updates coverage test setup after packTrust changes.
internal/runtime/capture_env.go Reworks runtime hook search roots to anchored locations; adds injectable providers for tests.
internal/runtime/capture_env_test.go Adds anchored-root test and utilities for overriding executable/caller providers.
internal/report/terminal.go Introduces terminal control-character sanitizer for report/table output.
internal/report/format_test.go Adds regression test ensuring table output escapes control characters.
internal/report/format_table.go Sanitizes string args emitted through table formatter helpers; sanitizes key row fields.
internal/report/format_table_values.go Sanitizes table value renderers that may include repo-controlled strings.
internal/report/format_table_sections.go Sanitizes additional table sections (scope, policy sources, comparison keys, codemod, warnings).
internal/notify/slack.go Escapes Slack top-level fallback text to prevent mrkdwn injection from repo names.
internal/notify/slack_test.go Adds tests covering fallback escaping for common mrkdwn/HTML-sensitive characters.
internal/notify/dispatcher.go Expands webhook URL redaction candidates to cover more real-world error-message variants.
internal/notify/dispatcher_test.go Adds tests for redacting path-escaped fragments, bare host+path, and bare token leakage.
internal/gitexec/gitexec.go Forces core.quotePath=false to reduce quoted-path surface area in git outputs.
internal/analysis/cache.go Treats symlinked cache roots as escape attempts (including broken symlinks) via Lstat precheck.
internal/analysis/cache_extra_test.go Adds regression test ensuring broken symlink cache paths are rejected and don’t create dirs outside repo.

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

Comment thread internal/runtime/capture_env.go
Comment thread internal/runtime/capture_env_test.go
@github-actions

Copy link
Copy Markdown
Contributor

Lopper (Delta)

Metric delta Value
Dependency count +0
Used percent +0.0%
Waste percent +0.0%
Estimated unused bytes +0 B
Known licenses +0
Unknown licenses +0
Denied licenses +0
Changed Regressions Progressions Added Removed Unchanged
0 0 0 0 0 9

No dependency-surface deltas detected.

@github-actions

Copy link
Copy Markdown
Contributor

SonarQube (PR)

Open issues: 0
Actionable issues shown (excluding mock/fixture files): 0

Duplication

  • Overall duplicated lines: 469
  • Overall duplication density: 0.40%
  • New duplicated lines: n/a
  • New duplication density: n/a

Issues

Open Sonar issues (0)
# Severity Rule Location Message
- - - - No open Sonar issues for this PR.

Source: SonarCloud PR view

@sonarqubecloud

Copy link
Copy Markdown

@ben-ranford ben-ranford merged commit 169988a into main Jun 10, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment