chore(release): prepare v2.6.0 pre-tag updates#641
Merged
Conversation
… + scan-cap + file_sentry hardening
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughv2.6.0 updates release docs and guides; adds Request Policy documentation and pipelock scan CLI doc; implements demo CLI Ed25519-signed receipts and receipt emission tests; introduces FileSentry max-file-size cap and watcher error reporting; coalesces config reloads to latest; Server.Reload defers ReverseProxy structural changes; distinguishes forward-proxy scan cap vs. data budget; and ungates Scan API tool_call Stage‑2 scanning when argument text exists. Changesv2.6.0 Release Documentation
Implementation and Testing
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
internal/filesentry/watcher_impl.go (1)
356-359:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSurface
os.Openfailures as well.Unreadable files still disappear silently here.
permission denied, transient removal, or other open-time failures never reach the new visibility path, so a watched file can still be left unscanned with no operator signal.Suggested fix
f, err := os.Open(filepath.Clean(path)) if err != nil { + w.logError(fmt.Errorf("filesentry: open failed, file left unscanned: %s: %w", path, err)) return }Also applies to: 427-430
🤖 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/filesentry/watcher_impl.go` around lines 356 - 359, The os.Open(filepath.Clean(path)) failure is being swallowed (if err { return }) — change this so open failures are surfaced instead of returning silently: either return the error up the call chain or call the watcher’s visibility/error-reporting API to record the path open failure (e.g., replace the silent return with a call that logs and records the error with the watcher/visibility component), and do the same change for the analogous os.Open handling at the other location (lines 427-430).internal/cli/diag/demo.go (1)
137-139:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRead interactive input from Cobra command input (
cmd.InOrStdin()), notfmt.Scanln()
fmt.Scanln()reads from process stdin, ignoringcmd.SetIn()/cmd.InOrStdin(), sopipelock demo --interactivecan’t be driven reliably by tests or scripted callers. Also ensure to add the neededbufioimport.Suggested change
+ reader := bufio.NewReader(cmd.InOrStdin()) + for i, s := range scenarios { if interactive && i > 0 { cmd.Print("\n Press Enter for next scenario...") - _, _ = fmt.Scanln() //nolint:errcheck // interactive prompt + _, _ = reader.ReadString('\n') } else if i > 0 { time.Sleep(150 * time.Millisecond) }🤖 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/cli/diag/demo.go` around lines 137 - 139, Replace the blocking fmt.Scanln() call (inside the interactive && i > 0 branch) with reading a line from the Cobra command input stream via cmd.InOrStdin() so tests and scripted callers work; create a bufio.Reader from cmd.InOrStdin(), call ReadString('\n') (or ReadBytes) to consume the Enter, handle or intentionally ignore the returned error instead of using fmt.Scanln(), and add the bufio import to the file. Ensure you still call cmd.Print("\n Press Enter for next scenario...") and reference the same interactive/i logic.
🧹 Nitpick comments (2)
internal/cli/diag/demo.go (2)
105-113: ⚡ Quick winNormalize
receiptsDirbefore using it for writes.This flag value is a trust boundary, but it is passed straight into
MkdirAlland later file writes. Clean it once up front and reuse the normalized path forsigner.puband per-receipt JSON files.Suggested change
func runDemo(cmd *cobra.Command, interactive, color bool, receiptsDir string) error { + if receiptsDir != "" { + receiptsDir = filepath.Clean(receiptsDir) + } + cfg := config.Defaults() cfg.Internal = nil // disable SSRF (avoids DNS lookups) @@ - rec := &demoReceipts{cmd: cmd, privKey: privKey, pubHex: pubHex, policyHash: policyHash, dir: receiptsDir, color: color} + rec := &demoReceipts{cmd: cmd, privKey: privKey, pubHex: pubHex, policyHash: policyHash, dir: receiptsDir, color: color}As per coding guidelines "In Go code, use filepath.Clean(path) to satisfy gosec G304 linter rule for file inclusion. For trust boundaries, also validate containment using EvalSymlinks and filepath.Rel."
Also applies to: 132-132, 274-275
🤖 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/cli/diag/demo.go` around lines 105 - 113, Normalize and validate the receiptsDir before creating or writing files: call filepath.Clean on receiptsDir at its earliest use, resolve symlinks with filepath.EvalSymlinks, and ensure the resolved path is contained within an expected base (use filepath.Rel to verify it doesn't escape), then reuse that normalized path for os.MkdirAll, for constructing pubPath (filepath.Join(normalizedReceiptsDir, "signer.pub")), and for per-receipt JSON writes; update the code around receiptsDir, os.MkdirAll, pubPath, and os.WriteFile to use the validated normalized path and return an error if EvalSymlinks or containment checks fail.
325-331: ⚡ Quick winUse shared severity constants instead of raw literals.
These receipts are release-facing metadata. Hardcoding
"high"and"critical"here makes the demo drift-prone if the canonicalconfig.Severity*values ever change.As per coding guidelines "Check existing config.Action*, config.Mode*, config.Severity* constants before creating new ones."
Also applies to: 344-350, 367-373, 384-390, 401-407, 440-446, 480-486
🤖 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/cli/diag/demo.go` around lines 325 - 331, Replace hardcoded severity string literals in the receipt definitions by using the shared config severity constants (e.g., change severity: "critical" and severity: "high" to severity: config.SeverityCritical and severity: config.SeverityHigh). Update every receipt literal that sets the severity field (including the examples around the "Credential Exfiltration" entry and the other ranges called out) to reference the canonical config.Severity* constants so these release-facing metadata values stay in sync with the central config.
🤖 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.
Inline comments:
In `@docs/compliance/eu-ai-act-mapping.md`:
- Line 11: The shorthand phrase "same five =" in the v2.6 review note (the line
starting with "v2.6 review (May 2026): per-frame WS request_policy,
allowlist_unparseable passthrough, media truncation, file_sentry block,
header-DLP parity, submit SSRF-safe dial = Art.15/15(4)/15(5); Conductor
enterprise+fleet-license gated, never inline.") should be replaced with explicit
mappings: list each feature mentioned (per-frame WS request_policy,
allowlist_unparseable passthrough, media truncation, file_sentry block,
header-DLP parity, submit SSRF-safe dial, Conductor enterprise+fleet-license
gated, never inline) on its own line and map it directly to the specific EU AI
Act article/paragraph (e.g., "per-frame WS request_policy → Art.15(1)/15(4)"),
mirroring the format used in prior sections so auditors can verify each claim
without inference; ensure consistency in wording and use the same article
identifiers used elsewhere in the document.
In `@docs/compliance/nist-800-53.md`:
- Line 11: Replace the compressed shorthand line that begins with "v2.6 review:
same five = AC-4(4),SI-4(4),SC-7,SI-12,SC-28,SI-10,AC-3,SC-7(5); Conductor
partially addresses CM-7/AU-12, fleet-license gated, never inline." with
explicit mappings: for each listed NIST control (e.g., AC-4(4), SI-4(4), SC-7,
SI-12, SC-28, SI-10, AC-3, SC-7(5), CM-7, AU-12) add a short sentence or bullet
that names the v2.6 capability it maps to and a one-line description of how that
capability satisfies the control (mention "Conductor" and "fleet-license" where
relevant), so auditors can see direct, reviewable traceability from v2.6
features to each NIST control.
In `@internal/filesentry/watcher_impl.go`:
- Around line 376-382: The read using io.ReadAll(io.LimitReader(f, sizeCap+1))
in watcher_impl.go currently ignores whether the extra byte was consumed, so if
the file grew after Stat() you may scan a truncated prefix; change the logic in
the scan path that uses io.ReadAll(io.LimitReader(f, sizeCap+1)) to check if
len(data) > sizeCap (i.e. the LimitReader returned the extra byte) and, when it
is, treat the file as grown/oversized/unscanned instead of proceeding to scan
the truncated data; apply the same fix to the equivalent block around the other
read (the second occurrence noted in the comment).
---
Outside diff comments:
In `@internal/cli/diag/demo.go`:
- Around line 137-139: Replace the blocking fmt.Scanln() call (inside the
interactive && i > 0 branch) with reading a line from the Cobra command input
stream via cmd.InOrStdin() so tests and scripted callers work; create a
bufio.Reader from cmd.InOrStdin(), call ReadString('\n') (or ReadBytes) to
consume the Enter, handle or intentionally ignore the returned error instead of
using fmt.Scanln(), and add the bufio import to the file. Ensure you still call
cmd.Print("\n Press Enter for next scenario...") and reference the same
interactive/i logic.
In `@internal/filesentry/watcher_impl.go`:
- Around line 356-359: The os.Open(filepath.Clean(path)) failure is being
swallowed (if err { return }) — change this so open failures are surfaced
instead of returning silently: either return the error up the call chain or call
the watcher’s visibility/error-reporting API to record the path open failure
(e.g., replace the silent return with a call that logs and records the error
with the watcher/visibility component), and do the same change for the analogous
os.Open handling at the other location (lines 427-430).
---
Nitpick comments:
In `@internal/cli/diag/demo.go`:
- Around line 105-113: Normalize and validate the receiptsDir before creating or
writing files: call filepath.Clean on receiptsDir at its earliest use, resolve
symlinks with filepath.EvalSymlinks, and ensure the resolved path is contained
within an expected base (use filepath.Rel to verify it doesn't escape), then
reuse that normalized path for os.MkdirAll, for constructing pubPath
(filepath.Join(normalizedReceiptsDir, "signer.pub")), and for per-receipt JSON
writes; update the code around receiptsDir, os.MkdirAll, pubPath, and
os.WriteFile to use the validated normalized path and return an error if
EvalSymlinks or containment checks fail.
- Around line 325-331: Replace hardcoded severity string literals in the receipt
definitions by using the shared config severity constants (e.g., change
severity: "critical" and severity: "high" to severity: config.SeverityCritical
and severity: config.SeverityHigh). Update every receipt literal that sets the
severity field (including the examples around the "Credential Exfiltration"
entry and the other ranges called out) to reference the canonical
config.Severity* constants so these release-facing metadata values stay in sync
with the central config.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 883e12e0-d907-43c7-96b4-682f0e2352a6
⛔ Files ignored due to path filters (1)
assets/demo.gifis excluded by!**/*.gif
📒 Files selected for processing (25)
CHANGELOG.mdREADME.mddocs/cli/scan.mddocs/compliance/eu-ai-act-mapping.mddocs/compliance/nist-800-53.mddocs/configuration.mddocs/guides/codex.mddocs/guides/health.mddocs/guides/posture-capsule.mddocs/specs/in-toto-agent-action-receipt-v0.1.mddocs/specs/pipelock-conductor-audit-sink.mdinternal/cli/diag/demo.gointernal/cli/diag/demo_test.gointernal/cli/runtime/server_reload.gointernal/cli/runtime/server_test.gointernal/config/canonical_golden_test.gointernal/config/reload.gointernal/config/reload_test.gointernal/config/schema.gointernal/filesentry/watcher_impl.gointernal/filesentry/watcher_test.gointernal/proxy/forward.gointernal/proxy/forward_test.gointernal/scanapi/handler_test.gointernal/scanapi/scan.go
Replace 1607 em-dash (U+2014) occurrences across 419 Go source files. Only comment em-dashes were changed; 247 string-literal em-dashes were left untouched for manual review.
…g polish Add the missing validation for file_sentry.max_file_bytes < 0. The schema comment and two docs already stated negative values are rejected at validation time, but validateFileSentry never enforced it (runtime fell back to the 10 MiB default, so the docs claimed enforcement the binary did not provide). Add the check plus negative-reject / zero-accept tests. Also: correct the scanner-pipeline list in CLAUDE.md (9 -> 11 layers, adding CRLF injection and path traversal to match scanner.go); note the demo scenario swap (high-entropy -> cloud-metadata SSRF) in the changelog; add the sessionKeyFor refactor to the changelog; add a Helm 0.2.0 digest-unpin upgrade note.
|
|
BlueAceFrost
previously approved these changes
May 31, 2026
BlueAceFrost
approved these changes
May 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
pipelock demowith signed action receipts and the cloud-metadata SSRF scenario, including the updated demo GIF.tool_callscanning, FileSentry oversized/unreadable skip visibility, and demo receipt/test hygiene.dns.host_overrides,fetch_proxy.monitoring.query_entropy_exclusions,reverse_proxy.profile: submitfields, Scan APItool_callbehavior,file_sentry.max_file_bytes,tool_chain_detection.sensitivity_labels, andlicense_crl_file.0.3.0/ appVersion2.6.0and clear the stale v2.5 default image digest so chart installs follow the v2.6.0 image tag unless operators explicitly pin a digest.Summary by CodeRabbit
New Features
Enhancements
Bug Fixes