Skip to content

Add Markdown output for artifact inspection - #248

Open
a3ylf wants to merge 1 commit into
git-pkgs:mainfrom
a3ylf:fix/inspect-markdown
Open

a3ylf wants to merge 1 commit into
git-pkgs:mainfrom
a3ylf:fix/inspect-markdown

Conversation

@a3ylf

@a3ylf a3ylf commented Nov 15, 2026 •

Copy link
Copy Markdown
Contributor

Closes #180.

brief inspect --markdown artifact rejects the output flag, while brief --markdown artifact drops it when automatically routing to inspection and produces JSON or human-readable output instead.

Add a Markdown artifact report and carry the flag through automatic inspection. Include archive counts, each native object's metadata, Go build information, and low-confidence static-library hints. Escape metadata and flatten embedded line breaks. Markdown takes precedence over JSON and human output, matching the other reporting commands.

Validation on Linux/amd64 with Go 1.27.3:

  • All six subprocess CLI regressions fail against the original code and pass with the fix, covering explicit/automatic inspection and output-flag precedence.
  • Report tests cover multiple objects, absent optional fields, empty artifacts, and escaping across all metadata fields.
  • go test -race ./..., go vet ./..., and the CLI build pass.
  • Configured golangci-lint 2.14.0: zero issues.
  • The rebuilt CLI produces Markdown with architecture and Go build metadata when inspecting a real native executable through either command path.

Summary by CodeRabbit

  • New Features
    • Added a --markdown option to brief inspect and direct artifact inspection to display artifact reports in Markdown.
    • Markdown reports include artifact details such as format, checksum, entries, native objects, and duration.
    • Documented Markdown output in the Inspect examples.

Render escaped artifact and native-object metadata in Markdown. Preserve the Markdown flag when automatically routing artifacts from the scan command, with the same output-format precedence as other commands.

Fixes git-pkgs#180.

Signed-off-by: Alexandre Rodrigues <alexandre3ylf@gmail.com>
@coderabbitai

coderabbitai Bot commented Nov 15, 2026 •

Copy link
Copy Markdown

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

brief inspect now supports Markdown artifact reports. Automatic inspection forwards the Markdown flag. The report includes artifact and native-object details, omits empty values, and escapes displayed metadata.

Changes: Markdown inspection

Layer / File(s) Summary
Artifact Markdown report
report/artifact.go, report/artifact_test.go
Adds ArtifactMarkdown to render artifact metadata, native-object details, and duration. Tests cover populated and empty reports, plus escaped metadata.
Inspect command integration
cmd/brief/inspect.go, cmd/brief/main.go, cmd/brief/inspect_test.go, README.md
Adds Markdown output selection to brief inspect and forwards the flag during automatic inspection. Tests check explicit and automatic inspection with Markdown flag combinations. README examples show both command forms.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Feature

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant PrimaryCLI
  participant cmdInspect
  participant ArtifactMarkdown
  participant stdout
  User->>PrimaryCLI: Request automatic inspection with --markdown
  PrimaryCLI->>cmdInspect: Forward -markdown for artifact inspection
  cmdInspect->>ArtifactMarkdown: Render the artifact report
  ArtifactMarkdown->>stdout: Write Markdown output
Loading

Suggested reviewers: andrew

Merge Risk: Minimal · up to 09456

This adds Markdown output for artifact inspection through both the explicit and automatic inspect commands. The change is additive, and the existing JSON and human-readable output paths remain available. No merge-blocking risk was identified.

Security architecture risk: Minimal · up to 09456

The new output format applies text sanitization and escaping without changing input processing or privileges. No introduced security or failure-containment regression was identified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed exposure is the Markdown report produced when the caller requests that format. The inspected comparison does not broaden the automatic-routing condition or input-processing authority; an attacker-controlled input can influence displayed metadata within that report.

Trust Boundaries and Controls

  • observed — Input-derived strings cross into Markdown through a single text helper. It replaces newline and tab characters with spaces, removes other control characters, escapes Markdown delimiters, and encodes angle brackets and ampersands. Dynamic headings and all metadata values use this helper, while field labels are fixed literals.
Pre-merge checks: Passed 4

Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title "Add Markdown output for artifact inspection" matches the main change. The PR adds an ArtifactMarkdown report function, a --markdown flag for brief inspect, forwarding of the flag through au…
Linked Issues check Passed Issue #180 (direct, open) requires: (1) a --markdown flag on cmdInspect; (2) report.ArtifactMarkdown(w, art); (3) --markdown forwarded through inspectAutoArgs on auto-routing; (4) Markdown p…
Out of Scope Changes check Passed All changed areas map to #180. README.md adds Markdown inspect examples, which document the new flag. cmd/brief/inspect.go adds the --markdown flag and its precedence. cmd/brief/main.go forwards the…
Full details: Title check

Explanation

The title "Add Markdown output for artifact inspection" matches the main change. The PR adds an ArtifactMarkdown report function, a --markdown flag for brief inspect, forwarding of the flag through automatic inspect routing, and tests for these paths. The title is concise, specific, and states the primary change, so it passes.

Full details: Linked Issues check

Explanation

Issue #180 (direct, open) requires: (1) a --markdown flag on cmdInspect; (2) report.ArtifactMarkdown(w, art); (3) --markdown forwarded through inspectAutoArgs on auto-routing; (4) Markdown precedence over JSON and human output; (5) artifact path, format, SHA-256, entry and native-object counts, and duration; (6) per-object path, format, architecture, SOName, producers, needed libraries, Go build info, and static-library hints; (7) escaping of metadata. Evidence at the reviewed head: cmd/brief/inspect.go defines markdownFlag and tests it first, so Markdown takes precedence over --json and --human. cmd/brief/main.go passes *markdownFlag into inspectAutoArgs(...) for auto-routed inspection. report/artifact.go implements ArtifactMarkdown, which writes the version, path, format, SHA-256, entry and object counts, a section per native object with format, architecture, SOName, producers, needed libraries, Go version, path, main package, and dependencies, static-library hints marked low confidence, and duration. Metadata passes through escapeCitationMarkdown(sanitizeLine(...)). Unit tests cover populated, empty, and escaping cases (per the summary). Caveats: the bodies of inspectAutoArgs, escapeCitationMarkdown, and sanitizeLine were not read directly; their behavior rests on the call sites and the change summary. The diffs for cmd/brief/inspect.go and main.go could not be read, so the head file contents were used. The report uses no Markdown tables, so table-pipe escaping is not structurally required; the escape helper covers Markdown-special characters. The CLI subprocess regression tests were not executed in this review.

Full details: Out of Scope Changes check

Explanation

All changed areas map to #180. README.md adds Markdown inspect examples, which document the new flag. cmd/brief/inspect.go adds the --markdown flag and its precedence. cmd/brief/main.go forwards the Markdown flag on automatic routing and still excludes scan-only flags. report/artifact.go adds ArtifactMarkdown. report/artifact_test.go and cmd/brief/inspect_test.go add tests for the new output and CLI routing. No change was found that lacks a connection to the issue. Caveat: the full diffs for cmd/brief/inspect.go and main.go were unavailable, so the check relied on head file contents and the change summary.

Finishing Touches

Generate unit tests (beta)

  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: d5df63cb-ef2d-46b2-8783-3ce485be4e27
Commits

Reviewing files that changed from the base of the PR and between 4ce3b27 and 0945694.

Files selected for processing: 6

  • README.md
  • cmd/brief/inspect.go
  • cmd/brief/inspect_test.go
  • cmd/brief/main.go
  • report/artifact.go
  • report/artifact_test.go

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.

@andrew andrew 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.

Thanks for this. The flag wiring and precedence match the other commands, and the subprocess tests covering both brief inspect --markdown and the auto-routed path are what I'd want. A few changes before it goes in.

report/artifact.go:21 prints the Entries line unconditionally, so a bare binary (brief --markdown build/tool) reports **Entries:** 0 files, 1 native objects. ArtifactHuman only prints it when a.Entries > 0. Please add the same guard and update TestArtifactMarkdownEmpty, which currently asserts the 0 files, 0 native objects string.

Producer, Needed and Go dependencies each get one bold paragraph per value, so a Go binary with 150 module deps renders 150 **Go dependency:** ... paragraphs per object. The rest of the Markdown output uses a **Header:** line followed by - item bullets capped at maxDisplayItems (mdTruncatedList in report/markdown.go). Please use that shape for these three, with the values still going through the escaping.

ArtifactMarkdown is in a new report/artifact.go, while ArtifactJSON and ArtifactHuman are in report/report.go and every other *Markdown function is in report/markdown.go. Please move it into markdown.go, and rename escapeCitationMarkdown to escapeMarkdown now that it has a caller outside the citation code.

sanitizeLine strips control characters but not Unicode format characters such as bidi overrides. The citation path strips unicode.Cf as well (report/citation.go). Paths, SONAMEs and producer strings from an untrusted archive should get the same treatment in the Markdown output, so please strip unicode.Cf in the text closure and add a case to the escaping test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add Markdown output support to brief inspect

2 participants