Skip to content

feat(audit): persist flash creation records#14

Draft
elibosley wants to merge 1 commit into
mainfrom
codex/persistent-flash-audit
Draft

feat(audit): persist flash creation records#14
elibosley wants to merge 1 commit into
mainfrom
codex/persistent-flash-audit

Conversation

@elibosley
Copy link
Copy Markdown
Member

@elibosley elibosley commented Apr 29, 2026

Summary

  • Write compact JSON audit records for create_flash_boot.sh and create_internal_boot_user.sh when installer persistence is mounted.
  • Capture action, success/failure status, selected Unraid ZIP filename and SHA256, target device details, and runtime override SHA256s.
  • Maintain flash-audit-latest.json and prune timestamped flash-audit-*.json records by count and total bytes to avoid filling persistence.
  • Document audit log location and retention controls in docs/USER_COMMANDS.md.

Why

Partners need a durable per-media audit trail for what the installer flashed, but the existing operation logs live under /tmp and do not survive reboot. These records are written to the installer media persistence partition, not the target Unraid server or newly created flash device.

Retention

Defaults:

  • INSTALLER_AUDIT_MAX_FILES=25
  • INSTALLER_AUDIT_MAX_BYTES=1048576

flash-audit-latest.json is a convenience copy of the latest record; pruning applies to timestamped audit records.

Validation

  • bash -n scripts/create_flash_boot.sh
  • bash -n scripts/create_internal_boot_user.sh
  • git diff --check HEAD~1..HEAD
  • shellcheck scripts/create_flash_boot.sh scripts/create_internal_boot_user.sh
  • Generated sample flash audit JSON and parsed it with python3 -m json.tool
  • Generated sample internal boot audit JSON and parsed it with python3 -m json.tool
  • Exercised audit pruning with INSTALLER_AUDIT_MAX_FILES=2 and verified flash-audit-latest.json is preserved

Summary by CodeRabbit

Release Notes

  • New Features

    • Flash creation operations now generate persistent JSON audit logs capturing action status, selected files, target device details, and configuration overrides.
    • Audit logs are automatically pruned based on configurable retention limits (max 25 files, 1 MiB storage).
  • Documentation

    • Added documentation for audit logging configuration and retention limit customization options.

- Purpose: give partners a durable per-media audit trail for flash creation actions.
- Before: flash and internal boot operations kept detailed operation logs under /tmp, which were useful during the session but did not persist for later partner review.
- Problem: partners could not reliably inspect installer media later to confirm which Unraid ZIP, target device, boot settings, or runtime overrides were used.
- Change: write compact JSON audit records under /mnt/persist/logs when installer persistence is mounted, including status, selected ZIP checksum, target details, and runtime override checksums.
- How: add audit writers to create_flash_boot.sh and create_internal_boot_user.sh, maintain flash-audit-latest.json, prune timestamped records by count and total bytes, and document the retention controls.
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 29, 2026

📝 Walkthrough

Walkthrough

These changes introduce persistent flash audit logging functionality that records flash creation operations as structured JSON entries when a persistent mount is available, capturing operation status, ZIP metadata, device details, and runtime override file checksums with configurable log retention policies.

Changes

Cohort / File(s) Summary
Documentation
docs/USER_COMMANDS.md
Adds documentation describing persistent flash audit logging, including stored data format (action, status, ZIP filename+SHA256, device details), log location (/mnt/persist/logs/), default retention limits (25 files, 1 MiB), and configuration environment variables (INSTALLER_AUDIT_MAX_FILES, INSTALLER_AUDIT_MAX_BYTES).
Flash Audit Logging Implementation
scripts/create_flash_boot.sh, scripts/create_internal_boot_user.sh
Implements audit logging functionality: initializes audit state before operations, captures success/failure outcomes with error details, generates structured JSON audit records containing schema/action/status/error, ZIP path+SHA256, disk/partition metadata, UEFI/bootable settings, and runtime override file SHA256s. Includes helper functions for JSON formatting, checksum computation, device metadata queries via lsblk/blkid, and audit log pruning based on size/file count limits. Updates ZIP selection method from ls to find for consistency.

Sequence Diagram

sequenceDiagram
    participant Script as Installation Script
    participant Audit as Audit System
    participant FS as Persistence Layer
    
    Script->>Audit: Initialize audit state (AUDIT_STARTED=1)
    activate Audit
    Note over Audit: Set default limits, prepare audit context
    
    Script->>Script: Perform flash creation operations
    
    alt Operation succeeds
        Script->>Audit: Collect metadata (ZIP, device, overrides)
        Audit->>Audit: Compute SHA256s, format JSON
        Audit->>FS: Write success audit record to /mnt/persist/logs/
    else Operation fails
        Script->>Audit: error_msg() updates AUDIT_ERROR
        Audit->>Audit: Collect metadata, format JSON with error
        Audit->>FS: Write failure audit record via EXIT trap
    end
    
    Audit->>FS: Prune old logs (enforce max files/bytes limits)
    deactivate Audit
    FS-->>FS: Remove excess timestamped audit files
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Poem

🐰 Hops through the flash with audits so keen,
JSON records in every scene,
SHA256s checksummed, status logged tight,
Old logs pruned with all their might,
The persistent layer shines ever bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% 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 follows the conventional commits format with 'feat:' prefix and clearly describes the main change: adding persistent flash creation audit records.
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

Review rate limit: 2/5 reviews remaining, refill in 24 minutes and 29 seconds.

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

@elibosley elibosley requested a review from SimonFair April 29, 2026 20:44
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/USER_COMMANDS.md`:
- Around line 186-203: Update the "Persistent Flash Audit Logs" section wording
to state that audit records under /mnt/persist/logs/ include both flash creation
and internal-boot creation actions (specifically mention create_internal_boot as
well as create_flash_boot.sh), so users know the timestamped files
(flash-audit-<timestamp>.json and flash-audit-latest.json) cover installer
actions for both flash and internal-boot; keep the existing details about
included fields, pruning defaults, and the INSTALLER_AUDIT_MAX_FILES /
INSTALLER_AUDIT_MAX_BYTES overrides but change any language that implies
"flash-only" to "installer actions (flash and internal-boot)".

In `@scripts/create_flash_boot.sh`:
- Around line 412-416: The audit filename generation (variables timestamp and
filename) uses only second-level precision and can collide with other scripts
writing the same pattern; change the filename generation to be
collision-resistant by appending a high-resolution timestamp or unique token
(e.g., include nanoseconds from date +%N, the process id $$, or a generated
UUID) or by creating a unique temporary name via mktemp and then moving it into
final; ensure you update tmp/final handling so the unique name is preserved and
latest still points to the correct file (adjust variables filename, tmp, final,
and latest accordingly).
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro (Legacy)

Run ID: 36869a57-5dfd-4ff2-8c0b-977f6e1c1c99

📥 Commits

Reviewing files that changed from the base of the PR and between a598445 and 7b7cae2.

📒 Files selected for processing (3)
  • docs/USER_COMMANDS.md
  • scripts/create_flash_boot.sh
  • scripts/create_internal_boot_user.sh

Comment thread docs/USER_COMMANDS.md
Comment on lines +186 to +203
## Persistent Flash Audit Logs

When persistence is mounted, flash creation writes compact JSON audit records to
the installer media under `/mnt/persist/logs/`.

- `flash-audit-<timestamp>.json`: timestamped audit record
- `flash-audit-latest.json`: copy of the latest audit record

Audit records include the flash action, status, selected Unraid ZIP filename and
SHA256, target device details, and runtime override SHA256s when override files
are present. They are written to the installer media persistence partition, not
to the target Unraid server or newly created flash device.

To avoid filling persistence, audit logs are pruned after writes. Defaults keep
at most 25 timestamped audit files and at most 1 MiB total timestamped audit log
data. Override with `INSTALLER_AUDIT_MAX_FILES` and
`INSTALLER_AUDIT_MAX_BYTES` if a partner workflow needs a different retention
window.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Document the internal-boot audit path too.

This section reads as flash-only, but the PR also persists create_internal_boot audit records into the same location. Update the wording so users know these files cover both installer actions, not just create_flash_boot.sh.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/USER_COMMANDS.md` around lines 186 - 203, Update the "Persistent Flash
Audit Logs" section wording to state that audit records under /mnt/persist/logs/
include both flash creation and internal-boot creation actions (specifically
mention create_internal_boot as well as create_flash_boot.sh), so users know the
timestamped files (flash-audit-<timestamp>.json and flash-audit-latest.json)
cover installer actions for both flash and internal-boot; keep the existing
details about included fields, pruning defaults, and the
INSTALLER_AUDIT_MAX_FILES / INSTALLER_AUDIT_MAX_BYTES overrides but change any
language that implies "flash-only" to "installer actions (flash and
internal-boot)".

Comment on lines +412 to +416
timestamp="$(date -u '+%Y-%m-%dT%H:%M:%SZ' 2>/dev/null || date)"
filename="flash-audit-$(date -u '+%Y%m%dT%H%M%SZ' 2>/dev/null || date '+%Y%m%dT%H%M%S').json"
final="$audit_dir/$filename"
tmp="$final.tmp"
latest="$audit_dir/flash-audit-latest.json"
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Use a collision-resistant audit filename.

This name only has second-level precision, and scripts/create_internal_boot_user.sh writes to the same directory with the same pattern. Two installer actions finishing in the same second will clobber one another and silently drop an audit record.

💡 Suggested fix
-    local audit_root audit_dir timestamp filename tmp final latest
+    local audit_root audit_dir timestamp tmp final latest
@@
-    filename="flash-audit-$(date -u '+%Y%m%dT%H%M%SZ' 2>/dev/null || date '+%Y%m%dT%H%M%S').json"
-    final="$audit_dir/$filename"
-    tmp="$final.tmp"
+    tmp="$(mktemp "$audit_dir/flash-audit-$(date -u '+%Y%m%dT%H%M%SZ' 2>/dev/null || date '+%Y%m%dT%H%M%S').XXXXXX.json.tmp")" || return 0
+    final="${tmp%.tmp}"
     latest="$audit_dir/flash-audit-latest.json"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/create_flash_boot.sh` around lines 412 - 416, The audit filename
generation (variables timestamp and filename) uses only second-level precision
and can collide with other scripts writing the same pattern; change the filename
generation to be collision-resistant by appending a high-resolution timestamp or
unique token (e.g., include nanoseconds from date +%N, the process id $$, or a
generated UUID) or by creating a unique temporary name via mktemp and then
moving it into final; ensure you update tmp/final handling so the unique name is
preserved and latest still points to the correct file (adjust variables
filename, tmp, final, and latest accordingly).

@elibosley
Copy link
Copy Markdown
Member Author

Release-readiness triage update on 2026-05-20:

Leaving this as a draft enhancement. Persistent flash audit records are useful partner-facing functionality, but this PR is not required for public repository visibility or the minimal/unseeded release path.

Before this should be considered merge-ready, it still needs to be refreshed against current main and the previous review feedback should be addressed.

@elibosley elibosley added the enhancement New feature or request label May 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant