refactor(proxy): consolidate adaptive-upgrade emissions; fix TMPDIR-dependent policy hash

[luckyPipewrench]

Follow-on to the sessionKeyFor extraction (#643), completing the proxy deny-path consolidation from the tech-debt sweep.

Adaptive-upgrade helper

Every adaptive-enforcement upgrade emitted an audit log line (LogAdaptiveUpgrade) immediately followed by a Prometheus counter (RecordAdaptiveUpgrade), duplicated across 28 call sites in the fetch, forward, CONNECT, WebSocket, and TLS-intercept paths. Each site recomputed the escalation label and repeated the from/to actions for both calls, so the audit trail and the metric could silently drift apart in a future edit. All 28 sites now route through one recordAdaptiveUpgrade helper fed by a single adaptiveUpgrade value, making log/metric consistency structural. The metrics handle is nil-safe so the TLS-intercept optional-proxy path keeps its exact prior behavior. A parity test covers the wiring and the nil-metrics case. No behavioral change to the deny paths: same emissions, same order, same arguments.

Canonical policy hash TMPDIR fix

The default mcp_tool_policy.quarantine_dir is derived from os.TempDir(), and it was flowing into the canonical policy hash. That made the hash depend on the ambient TMPDIR, so identical policy produced a different hash across environments. The operational quarantine path is now excluded from the policy view (alongside the other operational paths already excluded there), restoring the admission-grade property that identical policy yields an identical hash. The defaults golden hash is bumped accordingly; the rich-config golden is unchanged because that fixture pins its own quarantine_dir. A regression test asserts the hash is invariant across two different TMPDIR values, plus a noise-field table entry.

Lint hygiene

Three suppressions from the tech-debt sweep removed with real fixes: extract repeated test literals to consts (two nolint:goconst), use errors.Is for a sentinel comparison (nolint:errorlint), rename a snake_case test helper (nolint:revive).

Summary by CodeRabbit

• Bug Fixes

  • Policy canonicalization no longer depends on temporary-directory environment settings.

• Chores

  • Unified adaptive-enforcement escalation tracking so audit logs and metrics are consistently recorded.
  • Expanded tests for canonical policy hashing, adaptive-upgrade telemetry, and various enforcement edge cases.
  • Cleaned up and modernized tests (naming, shared test constants, and helper usage).

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1990d34a-e167-4409-ab9e-f70c92fa6d79

📥 Commits

Reviewing files that changed from the base of the PR and between f86b9c4 and 02c7228.

📒 Files selected for processing (7)

• internal/cli/contain/install_review_fixes_test.go
• internal/config/canonical.go
• internal/config/canonical_golden_test.go
• internal/config/canonical_test.go
• internal/envelope/signer_test.go
• internal/proxy/adaptive_upgrade.go
• internal/proxy/adaptive_upgrade_test.go

✅ Files skipped from review due to trivial changes (2)

• internal/config/canonical_golden_test.go
• internal/cli/contain/install_review_fixes_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

• internal/envelope/signer_test.go
• internal/config/canonical_test.go
• internal/proxy/adaptive_upgrade.go

---

📝 Walkthrough

Walkthrough

This PR consolidates adaptive-enforcement telemetry recording by introducing a recordAdaptiveUpgrade helper that unifies audit logging and metrics, excludes TMPDIR-derived quarantine paths from canonical policy hashing for environment independence, updates error handling to use errors.Is(), and deduplicates test constants.

Changes

Canonical Policy Hash Environment Independence

Layer / File(s)	Summary
Canonical view zeroing and fixture update internal/config/canonical.go, internal/config/canonical_golden_test.go	policySemanticView() clears MCPToolPolicy.QuarantineDir before hashing; goldenHashDefaults is updated and documented to reflect the environment-independent canonical hash.
Canonical hash environment-independence tests internal/config/canonical_test.go	Adds a noise-field case that mutates QuarantineDir and a TMPDIR-variance test that verifies Defaults() with different TMPDIR values produce identical canonical hashes.

Adaptive Upgrade Telemetry Consolidation

Layer / File(s)	Summary
Adaptive upgrade helper definition and tests internal/proxy/adaptive_upgrade.go, internal/proxy/adaptive_upgrade_test.go	New adaptiveUpgrade input type and recordAdaptiveUpgrade helper; tests verify audit/metric wiring and nil-metrics safety.
CONNECT and forward HTTP proxy escalation refactor internal/proxy/forward.go	Replaces direct logger/metrics calls with recordAdaptiveUpgrade across CONNECT and forward HTTP escalation points.
Interception handler escalation refactor internal/proxy/intercept.go	Replaces adaptive-upgrade logger/metrics pairs with recordAdaptiveUpgrade (conditional metrics capture) for URL, body, session-deny, and response-injection paths.
Fetch handler escalation refactor internal/proxy/proxy.go	Routes multiple fetch-path escalation telemetry through recordAdaptiveUpgrade, supplying session-level labels and scanner metadata.
WebSocket lifecycle escalation refactor internal/proxy/websocket.go	Swaps direct adaptive-upgrade logger/metrics calls for recordAdaptiveUpgrade across handshake, frame, body, in-stream relay, and response-injection stages.

Minor Test Improvements and Error Handling

Layer / File(s)	Summary
CLI subprocess helper rename internal/cli/contain/install_review_fixes_test.go	Renames exec_realCommand to execRealCommand and updates tests to call the new helper.
Sentinel error assertion improvement internal/envelope/signer_test.go	Imports errors and uses errors.Is() to assert the nil-signer error case.
Sentry test constant deduplication internal/sentry/scrub_test.go	Introduces testEnvSecret and testAWSKeyID constants and updates many tests to reference them instead of inline literals.

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly Related PRs

• luckyPipewrench/pipelock#425: Related to canonical-hash fixture pinning and golden updates connected to excluding MCPToolPolicy.QuarantineDir.
• luckyPipewrench/pipelock#643: Overlaps adaptive-enforcement telemetry call sites; that PR refactors the session key passed into telemetry records.
• luckyPipewrench/pipelock#290: Related prior refactor centralizing escalation/telemetry logic within internal/proxy/*.

Suggested Labels

size:XXL, pr:verified, contributor:verified

Suggested Reviewers

• DeathCamel58
• BlueAceFrost

Poem

🐰 A helper hops through proxy lanes,
Uniting logs with metrics' refrains,
Quarantine paths set free from hash,
Tests tidied up in a gentle dash,
Small fixes hop — the code sustains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 27.59% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the two main themes of the changeset: consolidating adaptive-upgrade emissions across multiple proxy paths and fixing the TMPDIR-dependent canonical policy hash issue.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/proxy-deny-consolidation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/config/canonical_golden_test.go (1)

208-208: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Don’t bump goldenHashRichConfig; QuarantineDir is excluded from the canonical policy hash view
MCPToolPolicy.QuarantineDir defaults to an os.TempDir()-derived path, but internal/config/canonical.go explicitly removes it from the canonical hashing view (view.MCPToolPolicy.QuarantineDir = ""). Therefore omitting mcp_tool_policy.quarantine_dir in the rich YAML fixtures shouldn’t change TestCanonicalPolicyHash_GoldenRichConfig—the PR rationale should be updated to reflect the exclusion (not “fixture pins its own quarantine_dir”).

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/config/canonical_golden_test.go` at line 208, The golden hash was
incorrectly updated—do not change goldenHashRichConfig;
MCPToolPolicy.QuarantineDir is explicitly excluded from the canonical hashing
view (see internal/config/canonical.go where view.MCPToolPolicy.QuarantineDir =
""), so omitting mcp_tool_policy.quarantine_dir in rich YAML should not alter
TestCanonicalPolicyHash_GoldenRichConfig; revert the bump to
goldenHashRichConfig, restore the original expected hash, and update the PR
description to state that QuarantineDir is excluded from the canonical policy
hash rather than claiming the fixture pins its own quarantine_dir.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@internal/config/canonical_golden_test.go`:
- Line 208: The golden hash was incorrectly updated—do not change
goldenHashRichConfig; MCPToolPolicy.QuarantineDir is explicitly excluded from
the canonical hashing view (see internal/config/canonical.go where
view.MCPToolPolicy.QuarantineDir = ""), so omitting
mcp_tool_policy.quarantine_dir in rich YAML should not alter
TestCanonicalPolicyHash_GoldenRichConfig; revert the bump to
goldenHashRichConfig, restore the original expected hash, and update the PR
description to state that QuarantineDir is excluded from the canonical policy
hash rather than claiming the fixture pins its own quarantine_dir.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4c999d16-9ae4-40f2-909d-3245b33c0a06

📥 Commits

Reviewing files that changed from the base of the PR and between cfa0ae1 and f86b9c4.

📒 Files selected for processing (12)

• internal/cli/contain/install_review_fixes_test.go
• internal/config/canonical.go
• internal/config/canonical_golden_test.go
• internal/config/canonical_test.go
• internal/envelope/signer_test.go
• internal/proxy/adaptive_upgrade.go
• internal/proxy/adaptive_upgrade_test.go
• internal/proxy/forward.go
• internal/proxy/intercept.go
• internal/proxy/proxy.go
• internal/proxy/websocket.go
• internal/sentry/scrub_test.go

[sentry]

Codecov Report

❌ Patch coverage is 77.50000% with 9 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
internal/proxy/intercept.go	66.66%	4 Missing ⚠️
internal/proxy/forward.go	70.00%	3 Missing ⚠️
internal/proxy/proxy.go	80.00%	1 Missing ⚠️
internal/proxy/websocket.go	88.88%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!