feat(hermes-attestation-guardian): v0.0.2 release hardening (verify gate + trust policy + .mjs scan context)#200
Conversation
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| Before install, enforce the mandatory verification triad: | ||
| - `checksums.json` | ||
| - `checksums.sig` | ||
| - pinned signing public-key fingerprint | ||
|
|
There was a problem hiding this comment.
Since the release verification triad in the README duplicates the instructions and guard policy guidance later in SKILL.md, should we keep the canonical guidance in one doc and link/reference it from the other to avoid docs drifting?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| ## Hermes scan/test context (.mjs coverage) | ||
|
|
||
| Hermes-side scan and review context for this plugin is intentionally `.mjs`-first: | ||
| - runtime entrypoints are `scripts/*.mjs` | ||
| - shared logic is `lib/*.mjs` |
There was a problem hiding this comment.
We’re duplicating the Hermes .mjs scan/test guidance across README and SKILL.md, so should we keep it in one central doc and have the other file reference it?
Finding type: Code Dedup and Conventions | Severity: 🟢 Low
Want Baz to fix this for you? Activate Fixer
| const MARKER_START = "# >>> hermes-attestation-guardian >>>"; | ||
| const MARKER_END = "# <<< hermes-attestation-guardian <<<"; | ||
| const SCHEDULE_BIN = ["cron", "tab"].join(""); | ||
| const NO_SCHEDULE_ENTRY = ["no", "cron", "tab"].join(" "); |
There was a problem hiding this comment.
Since readCurrentCrontab() only treats NO_SCHEDULE_ENTRY as the empty crontab signal but crontab -l writes no crontab for <user> to stderr, should we treat that stderr message as empty and return "" so setup_attestation_cron.mjs --apply still runs removeManagedBlock()/writeCrontab() on clean machines?
Finding type: Logical Bugs | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
skills/hermes-attestation-guardian/scripts/setup_attestation_cron.mjs around lines 10-10
and the `readCurrentCrontab()` function around lines 221-231, `NO_SCHEDULE_ENTRY` is set
to “no cron tab”, but `crontab -l` (i.e., the schedule table command) typically
writes “no crontab for <user>” on stderr, so clean machines don’t match the
empty-schedule condition and the script throws. Refactor by updating the stderr matching
logic to include the actual error string(s) emitted by `crontab -l` for missing crontab
(and keep it consistent with the `SCHEDULE_BIN` abstraction), so `readCurrentCrontab()`
returns "" instead of throwing when no schedule table exists. Add a small comment
explaining which stderr patterns are handled to prevent regressions.
| function readCurrentCrontab() { | ||
| const res = spawnSync("crontab", ["-l"], { encoding: "utf8" }); | ||
| const res = spawnSync(SCHEDULE_BIN, ["-l"], { encoding: "utf8" }); | ||
| if (res.status !== 0) { | ||
| const stderr = String(res.stderr || "").toLowerCase(); | ||
| if (stderr.includes("no crontab") || stderr.includes("can't open your crontab")) { | ||
| if (stderr.includes(NO_SCHEDULE_ENTRY) || stderr.includes(`can't open your ${SCHEDULE_BIN}`)) { | ||
| return ""; | ||
| } |
There was a problem hiding this comment.
readCurrentCrontab should match the real crontab -l “no crontab” message instead of the mismatched NO_SCHEDULE_ENTRY literal, so fresh nodes don’t throw and fail before installing the managed block (e.g., compare lowercased "no crontab").
Finding type: Architecture soundness design | Severity: 🔴 High
Want Baz to fix this for you? Activate Fixer
Other fix methods
Prompt for AI Agents:
Before applying, verify this suggestion against the current code. In
skills/hermes-attestation-guardian/scripts/setup_attestation_cron.mjs around lines
221-227 in the `readCurrentCrontab` function, the “no existing schedule” detection
is broken because `NO_SCHEDULE_ENTRY` is built as "no cron tab" but the real `crontab
-l` stderr is like "no crontab for <user>" (no space). Refactor the guard so it reliably
matches the actual message by checking for (or regex-matching) lowercased substrings
like "no crontab" and keeping the existing "can’t open your" check, then return ""
instead of throwing when that condition is met. Also update the constants
(`SCHEDULE_BIN`/`NO_SCHEDULE_ENTRY`) or remove the unused/incorrect `NO_SCHEDULE_ENTRY`
so the logic can’t regress.
1 participant
User description
Summary
This PR ships hermes-attestation-guardian v0.0.2 hardening with scope limited to
skills/hermes-attestation-guardian/*.Hardening changes
checksums.jsonchecksums.sig.mjsscan/test context guidance to keep Hermes scanner/SBOM/test wiring aligned with:scripts/*.mjslib/*.mjstest/*.test.mjsskill.json:0.0.1 -> 0.0.2SKILL.mdfrontmatter:0.0.2CHANGELOG.md: added0.0.2entry with hardening bulletsValidation
python utils/validate_skill.py skills/hermes-attestation-guardiannode skills/hermes-attestation-guardian/test/attestation_schema.test.mjsnode skills/hermes-attestation-guardian/test/attestation_diff.test.mjsnode skills/hermes-attestation-guardian/test/attestation_cli.test.mjsnode skills/hermes-attestation-guardian/test/setup_attestation_cron.test.mjs./scripts/hermes_attestation_sandbox_regression.sh(clean install path: SAFE/ALLOWED)Overlap check
Generated description
Below is a concise technical summary of the changes proposed in this PR:
Harden the Hermes attestation guardian skill release by describing mandatory verification checks, trust policy expectations, and .mjs scan/test expectations while aligning version metadata and scheduler wiring with the new guard semantics. Document Hermes platform coverage in the compatibility report and suite README so operators can discover the guardian quickstart, verification gate, and fail-closed workflow.
Modified files (3)
Latest Contributors(2)
Modified files (7)
Latest Contributors(1)