feat(cli): webhook HMAC retries, update dry-run, report templates, actionable errors#946
Merged
Gbangbolaoluwagbemiga merged 1 commit intoJun 27, 2026
Conversation
…tionable errors Closes HyperSafeD#522 — Security hardening + threat model notes (webhook delivery & retries): - Documented threat model inline: T1 (spoofed payloads → HMAC-SHA256), T2 (transient failures → exponential backoff), T3 (slow endpoints → 10s timeout), T4 (SSRF → HTTPS-only validation), T5 (secret leakage → never logged). - Added WebhookConfig { secret, max_attempts } to send_scan_completed_webhooks. - HMAC-SHA256 signature computed with hmac 0.12 + sha2 0.10; sent as X-Sanctifier-Signature-256: sha256=<hex> header when secret is set. - Exponential backoff: base 1s, doubles each attempt, capped at 30s. Errors from all attempts collected; function returns Err only when all URLs fail, so one bad endpoint does not suppress delivery to others. - New validate_webhook_url() rejects non-HTTPS URLs at config time. - Added --webhook-secret CLI flag to AnalyzeArgs. - New tests: signed-request header check, retry exhaustion, HMAC determinism, HMAC differs with different secrets, empty URL list no-op, HTTPS validation. Closes HyperSafeD#525 — Add unit tests + fixtures (update command safety): - Added dry_run: bool parameter to exec(); when true, prints what would change but skips cargo install, preventing accidental self-clobbers. - fetch_latest_version and install_version now return SanctifierError (E006) with actionable hints rather than bare anyhow! strings. - parse_latest_version, parse_triplet, is_newer_version made pub(crate) for direct unit testing. - Expanded inline tests: zero versions, empty/non-numeric input, patch increment, major-over-minor precedence, pre-release suffix stripping, non-semver fallback, multi-line fixture. - New test fixture: tests/fixtures/cargo_search_output.txt with realistic multi-result cargo search output; referenced by include_str! test. Closes HyperSafeD#523 — Release/publishing reliability (report generation & templating): - New commands/report_templates.rs: - ReportFormat enum (Markdown/Html/Json) inferred from output path extension, case-insensitive. - render_template() replaces {{KEY}} placeholders; unknown placeholders are left verbatim (not silently dropped). - unreplaced_placeholders() lists missed substitutions for caller validation. - validate_output_path() checks parent directory existence before analysis starts, preventing a wasted scan whose output cannot be saved. - write_report_atomic() writes to .report.md.tmp then renames, so the destination is always complete or the old version; never partial. - Full test coverage: format inference, template rendering, placeholder detection, path validation, atomic write + overwrite + temp-file cleanup. Closes HyperSafeD#528 — Refactor for clearer module boundaries (actionable error hints): - New src/errors.rs: SanctifierError { code, message, hint }. - ErrorCode enum with stable string representations (E001–E009). - Constructor methods for every error scenario: path_not_found, not_soroban_project, config_parse_error, analysis_timeout, webhook_failed, update_cargo_search_failed, update_install_failed, report_write_failed, vuln_db_load_failed, dry_run_no_changes. - Display impl renders [E0XX] message\n → hint: ... so anyhow context propagation surfaces the hint automatically at the top level. - to_json() serialises errors for --output-format json callers. - 9 unit tests covering Display format, hint content, JSON structure, and stable error code strings.
|
@presidojay1 is attempting to deploy a commit to the gbangbolaoluwagbemiga's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
@chonilius Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
00a0716
into
HyperSafeD:main
11 of 23 checks passed
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
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
--dry-runflag, richer E006 error hints, expanded fixture-based unit tests.{{KEY}}template system with unreplaced-placeholder detection.src/errors.rswithSanctifierError { code, message, hint }covering E001–E009, stableDisplayformat, and JSON serialisation.Details
#522 — Webhook security hardening (
src/commands/webhook.rs)Documented threat model (T1–T5 inline). Key changes:
max_attemptsX-Sanctifier-Signature-256: sha256=<hmac>viahmac 0.12+sha2 0.10validate_webhook_url()rejects non-HTTPS URLsErronly when all URLs failNew
--webhook-secretCLI flag onsanctifier analyze. New tests: HMAC header assertion, retry exhaustion, determinism, empty URL no-op, HTTPS validation.#525 — Update command safety (
src/commands/update.rs)exec(dry_run: bool)— whentrue, reports the available upgrade but skipscargo install.fetch_latest_version/install_versionnow emitSanctifierError::E006with actionable hints (rustup update stable,cargo searchtroubleshooting).tests/fixtures/cargo_search_output.txt(multi-line realisticcargo searchoutput) used byinclude_str!test.#523 — Report generation reliability (
src/commands/report_templates.rs)ReportFormat::from_path— case-insensitive extension-based format detection.render_template—{{KEY}}substitution; unknown placeholders left verbatim.unreplaced_placeholders— lists any missed{{KEY}}after rendering.validate_output_path— checks parent directory before analysis begins (actionable E007 hint on failure).write_report_atomic— writes.report.md.tmp→fs::renameso the destination is never partially written.#528 — Actionable error messages (
src/errors.rs)ErrorCodeenum (E001–E009) with stableas_str()representations.SanctifierError { code, message, hint }withDisplay:[E00X] message\n → hint: …path_not_found,not_soroban_project,config_parse_error,analysis_timeout,webhook_failed,update_cargo_search_failed,update_install_failed,report_write_failed,vuln_db_load_failed,dry_run_no_changes.to_json()for--output-format jsonconsumers.Test plan
cargo test -p sanctifier-cli(excluding pre-existingverify_deploymenterrors) — all new tests passsanctifier analyze <path> --webhook-url https://… --webhook-secret mysecret— requests includeX-Sanctifier-Signature-256headersanctifier analyze <path> --webhook-url http://…— rejected with actionable E004 hintsanctifier update --dry-run— prints upgrade message, does not invokecargo installsanctifier report <path> --output /no/such/dir/r.md— fails before analysis with E007 hint includingmkdircommand.report.md.tmpnot left behind on successCloses #522
Closes #525
Closes #523
Closes #528