Skip to content

feat(blockreason): X-Pipelock-Block-Reason header schema and emit package#467

Open
luckyPipewrench wants to merge 3 commits intomainfrom
feat/block-reason-header
Open

feat(blockreason): X-Pipelock-Block-Reason header schema and emit package#467
luckyPipewrench wants to merge 3 commits intomainfrom
feat/block-reason-header

Conversation

@luckyPipewrench
Copy link
Copy Markdown
Owner

@luckyPipewrench luckyPipewrench commented May 2, 2026

Summary

Defines the v1 vocabulary for X-Pipelock-Block-Reason, the response header set agents read to decide whether to retry, switch tools, or surface a useful error to the user. Lands the schema doc and the emit package now so the vocabulary can be reviewed before any block site commits to it. The transport refactor that wires every block path to call Info.SetHeaders follows in a separate PR.

The header set:

  • X-Pipelock-Block-Reason (required): machine-readable code. 28 codes across egress, content, MCP, posture, and generic.
  • X-Pipelock-Block-Reason-Version (required): schema version.
  • X-Pipelock-Block-Reason-Severity (required): matches the existing internal/config/schema.go vocabulary (info, warn, critical).
  • X-Pipelock-Block-Reason-Retry (required): retry hint (none, transient, policy).
  • X-Pipelock-Block-Reason-Layer (optional): reuses internal/scanner/ Scanner* constants verbatim so operators can correlate header-driven agent behavior with existing audit logs and Prometheus labels.
  • X-Pipelock-Block-Reason-Receipt (optional): receipt UUID.

Why design first

Once an agent in production reads dlp_match, the vocabulary is locked. Renaming a code in v2 breaks every consumer. Splitting the schema and the transport refactor into separate PRs gives reviewers a chance to challenge the vocabulary before any of the roughly 50 block sites commits to it.

Privacy

The Info struct deliberately exposes only Reason, Severity, Retry, Layer, and Receipt. None of those fields can carry matched content, pattern names, or agent identifiers. Tests pin the surface so a future field addition trips review.

RFC 6455 floor

CloseFramePayload() is bounded to RFC 6455's 123-byte close-frame limit. Optional fields drop in a documented order (receipt, layer, retry, severity, version). If even the bare {"block_reason":"<code>"} would overflow because the Reason value is unusually long, the helper returns a fixed sentinel rather than a malformed close frame. Test invariant pins the size ceiling across normal, truncated, overlong-reason, and floor cases.

Anti-scope-creep

  • No call sites yet. The spec status block at the top of the doc says so explicitly. Transport PR follows.
  • No JSON-RPC error vocabulary for MCP stdio. Tracked separately; stdio has no HTTP layer to attach headers to.
  • No agent-side retry orchestration. Pipelock emits, the agent decides.

Summary by CodeRabbit

Release Notes

  • New Features

    • Added block reason metadata for HTTP responses and WebSocket connections, providing detailed information about blocked requests including reason, severity, and retry guidance.
  • Documentation

    • Added specification for X-Pipelock-Block-Reason header format and WebSocket close-frame behavior.
  • Tests

    • Added comprehensive test coverage for block reason functionality.

…kage

Defines the v1 vocabulary for machine-readable block reasons so agents can react intelligently to a block instead of treating every 403 as opaque. Locks header names, reason codes (28 across egress, content, MCP, posture, generic), severity values matching pipelock's existing config severity vocabulary, and retry hints (none, transient, policy).

Package emits four required headers (reason, version, severity, retry) and two optional (layer, receipt). CloseFramePayload helper fits within RFC 6455's 123-byte close-frame limit by dropping optional fields in a documented order; block_reason is always present.

Privacy invariant: the Info struct exposes no fields that could carry matched content, pattern names, or agent identifiers. Coverage 97.1% (mustMarshal error sentinel is defensive code unreachable through the public API given the fixed-shape struct).

Spec doc at docs/specs/block-reason-header.md is the canonical schema. Implementation across the six transports lands separately so the schema can be reviewed before any block site commits to it.
…, align layer labels

Review feedback on the initial schema. Three contract issues that would have shipped wrong if the transport PR followed:

1. Required headers were not required. SetHeaders skipped severity/retry on empty values and emitted only version on empty reason. New(reason, severity, retry) now enforces the triple at construction (panic on empty — programming error per the codebase's panic policy). SetHeaders unconditionally emits all four required headers. WithLayer and WithReceipt return copies for optional fields.

2. CloseFramePayload could return RFC 6455-overlong output. Now guaranteed to fit within 123 bytes. If even the bare {block_reason: <code>} would overflow, returns a fixed sentinel (parse_error / version 1) so the close frame is always RFC 6455-compliant. Test invariant pins len(out) <= closeFrameMaxBytes for every input shape.

3. Layer-label vocabulary diverged from existing scanner labels. Spec now reuses scanner.Scanner* constants verbatim (blocklist, ratelimit, length, databudget, dlp, ssrf, entropy) so operators can correlate the header with their existing audit / metrics streams. Reason codes stay user-facing snake_case (domain_blocklist, rate_limit, etc.); the mapping table makes both surfaces explicit.

Spec also reworded to flag PR-A status: vocabulary is the target; the transport PR will wire every block path.

Coverage: 97.3% (every public function 100%, mustMarshal defensive-error sentinel uncovered).
@brin-security-scanner brin-security-scanner Bot added contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis. labels May 2, 2026
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 2, 2026

Warning

Rate limit exceeded

@luckyPipewrench has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 25 minutes and 36 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: d6ffc079-84cb-4d36-8c32-c1d4ba3a1449

📥 Commits

Reviewing files that changed from the base of the PR and between 28c47c5 and 430f847.

📒 Files selected for processing (3)
  • docs/specs/block-reason-header.md
  • internal/blockreason/blockreason.go
  • internal/blockreason/blockreason_test.go
📝 Walkthrough

Walkthrough

Introduces a machine-readable block-reason metadata system with a canonical v1 HTTP/WebSocket specification and a Go implementation package that constructs, encodes, and serializes block reason triples (reason, severity, retry) to HTTP headers and WebSocket close frames with RFC 6455 byte-limit enforcement.

Changes

Block Reason Specification & Implementation

Layer / File(s) Summary
Specification
docs/specs/block-reason-header.md
Canonical v1 schema defining required headers (X-Pipelock-Block-Reason, -Version, -Severity, -Retry), optional headers (-Layer, -Receipt), reason/severity/retry vocabularies, layer-to-constant mappings, privacy constraints, versioning policy, and WebSocket close-frame JSON shape with RFC 6455 truncation rules.
Type & Constant Definitions
internal/blockreason/blockreason.go (lines 21–113)
Exports Reason, Severity, and Retry string types; HTTP header name constants; schema version constant; and full vocabulary of reason codes (SchemeBlocked, DomainBlocklist, SSRFPrivateIP, …, BadRequest), severity values (info, warn, critical), and retry hints (none, transient, policy).
Core Implementation
internal/blockreason/blockreason.go (lines 114–268)
Info struct holds required triple and optional Layer/Receipt fields; New() constructor panics on empty required fields; WithLayer() and WithReceipt() provide fluent configuration; SetHeaders() writes required headers always and optional headers conditionally; CloseFramePayload() generates JSON payload with iterative field-drop fallback to enforce 123-byte RFC 6455 limit; mustMarshal() defensively marshals with overflow sentinel.
Comprehensive Tests
internal/blockreason/blockreason_test.go
Validates constructor behavior and panic conditions, fluent chaining, required vs. optional header emission, full JSON payload shape, zero-info sentinel, RFC 6455 byte-limit enforcement across multiple payloads, overflow fallback, progressive field truncation, constant vocabulary (snake_case, expected string mappings), privacy (no secret leakage), and JSON round-trip correctness.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Block reasons bloom in headers fair,
With reason, severity, care—
WebSocket frames squeezed just right,
Truncated fields, tests shining bright,
Metadata hops through RFC's flight!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 11.11% 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 pull request title clearly and specifically summarizes the main change: introducing a new X-Pipelock-Block-Reason header schema specification and an emit package for that header set.
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 unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/block-reason-header

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
Review rate limit: 0/1 reviews remaining, refill in 25 minutes and 36 seconds.

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

@sentry
Copy link
Copy Markdown

sentry Bot commented May 2, 2026

Codecov Report

❌ Patch coverage is 97.77778% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
internal/blockreason/blockreason.go 97.77% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@luckyPipewrench luckyPipewrench self-assigned this May 2, 2026
@luckyPipewrench
Copy link
Copy Markdown
Owner Author

/review deep

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 2, 2026

AI Review: security deep (/review deep)

Model: gpt-5.4


  1. severity: medium
    file: internal/blockreason/blockreason.goInfo.SetHeaders, Info.CloseFramePayload, WithLayer, WithReceipt
    why it matters:
    The package documentation says callers “MUST use New” and that Layer/Receipt must not carry sensitive or user-attributable data, but the implementation does not enforce any of that. Any call site can:

    • construct Info{} directly,
    • set arbitrary Layer/Receipt strings,
    • emit empty required headers,
    • or place attacker-controlled strings into HTTP headers / WS close payloads.

    In this PR the package is not yet wired into enforcement, but once adopted this becomes a trust boundary: block metadata crosses from internal enforcement logic to agent-visible protocol surfaces. Relying on comments alone is brittle and creates a future leak/audit-integrity risk, especially for Receipt and Layer, which are explicitly intended to correlate with internal receipts/logs.
    concrete fix or safer pattern:
    Make invalid states unrepresentable:

    • unexport Info fields and expose only constructors/builders,
    • validate Reason/Severity/Retry against an allowlist, not just non-empty,
    • validate Layer against the known scanner constants,
    • validate Receipt format/charset/length (e.g. UUID/ULID only),
    • reject or sanitize control characters before writing headers / close payloads,
    • consider SetHeaders(...) error / CloseFramePayload() (string, error) or a Must... variant only for internal trusted paths.
  2. severity: medium
    file: internal/blockreason/blockreason.goNew
    why it matters:
    New panics on empty required fields. This is framed as a programming-error guard, but this helper is intended to sit on “every pipelock block path,” i.e. directly in enforcement/error handling. A panic in a block path can turn a safe deny into a process crash or connection abort, which is an operational safety issue and can weaken enforcement availability under edge cases or partial integrations. Security controls should fail closed without risking service instability.
    concrete fix or safer pattern:
    Replace panic-based construction with explicit validation:

    • func New(...) (Info, error) and propagate the error,
    • or provide fixed predefined Info values / factory funcs per reason so callers cannot mismatch fields,
    • if you keep a panicing helper, add a non-panicking validated path for runtime enforcement code and reserve panic for tests/init-time constants only.
  3. severity: low
    file: docs/specs/block-reason-header.mdX-Pipelock-Block-Reason-Receipt / Privacy / Transport coverage
    why it matters:
    The spec says the receipt header “carries a UUID,” while examples use 01HZ8..., which looks ULID-like, and the code accepts any string. For an audit/correlation identifier that may be consumed by agents and later used to fetch receipts, ambiguity in format and lack of normalization can create interoperability and auditability problems, and increases the chance that future call sites stuff non-opaque or user-derived identifiers into the field.
    concrete fix or safer pattern:
    Tighten the contract in both docs and code:

    • pick one opaque ID format (UUIDv4, ULID, etc.),
    • document exact allowed charset/length,
    • validate it in WithReceipt,
    • and reject anything else so the header remains a safe, opaque correlation handle.
  4. severity: low
    file: internal/blockreason/blockreason.goCloseFramePayload / closeFrameOverflowFallback
    why it matters:
    When the payload overflows, the function silently rewrites the reason to parse_error. That preserves RFC compliance, but it also destroys the original security signal. For incident response and agent behavior, misclassifying an actual tool_poisoning/prompt_injection/ssrf_* block as parse_error weakens audit fidelity and can lead to incorrect retry or remediation behavior. Today this only happens for malformed/overlong inputs, but the helper explicitly allows arbitrary Reason values, so the downgrade path is reachable if a future caller passes unvalidated data.
    concrete fix or safer pattern:
    Avoid semantic substitution:

    • validate Reason against the fixed enum up front so it can never be overlong,
    • or fall back to a dedicated stable code like block_reason_overflow / unknown_block,
    • and log/emit an internal audit event whenever truncation/fallback occurs so operators can investigate malformed metadata generation.

… ULID receipt

Address GPT review on the schema PR. Four items, all medium-or-lower severity but all real once the package is wired into enforcement.

1. Make invalid states unrepresentable. New now validates reason, severity, and retry against fixed allowlists (validReasons, validSeverities, validRetries) and returns ErrInvalidReason / ErrInvalidSeverity / ErrInvalidRetry rather than accepting any non-empty string. WithLayer restricts to ASCII alphanumeric and underscore (the shape of internal/scanner/Scanner* constants), bounded by layerMaxLen. WithReceipt restricts to a 26-char Crockford-base32 ULID, locking the slot opaque so attacker-controlled metadata cannot reach agent-visible response headers.

2. New no longer panics on the enforcement hot path. Returns Info, error so callers can fail closed without crashing the process. MustNew kept as the panicking variant reserved for compile-time-known constants and tests.

3. Receipt format locked to ULID. Spec now documents the exact charset and length (26 chars, Crockford-base32: 0-9 plus A-Z minus I, L, O, U). Validator rejects any non-conforming string.

4. CloseFramePayload overflow no longer downgrades to parse_error. New BlockReasonOverflow code preserves the audit signal that block-emit metadata was itself malformed, rather than silently reclassifying the original block.

Coverage 98.5 percent. mustMarshal defensive sentinel uncovered (unreachable through public API).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

contributor:verified Contributor passed trust analysis. pr:verified PR passed security analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant