Add daily Sentinel v2.4 operational-check script, runbook, and tests#142
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
The files' contents are under analysis for test generation. |
Changed Files
|
|
Review these changes at https://app.gitnotebooks.com/OneFineStarstuff/OneFineStarstuff.github.io/pull/142 |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
View changes in DiffLens |
Reviewer's GuideAdds a new daily Sentinel v2.4 operational-check CLI that validates dashboard reachability, G-SRI evidence, PQC WORM Object Lock commits, and TEE/TPM attestation; wires it into the project entry points, documents runbook usage and expectations, and provides pytest coverage for core check logic and CLI behavior. Sequence diagram for daily Sentinel v2.4 operational-check CLI flowsequenceDiagram
actor Operator
participant daily_sentinel_operational_check_main as main
participant SentinelDashboard
participant EvidenceFS
Operator->>daily_sentinel_operational_check_main: main(argv)
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: parse_args(argv)
alt dashboard_not_skipped
daily_sentinel_operational_check_main->>SentinelDashboard: check_dashboard(dashboard_url, dashboard_timeout, insecure_tls)
SentinelDashboard-->>daily_sentinel_operational_check_main: HTTP status code
else dashboard_skipped
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: CheckResult("sentinel_dashboard", WARN, "dashboard reachability skipped by operator")
end
daily_sentinel_operational_check_main->>EvidenceFS: load_json(gsri_evidence_path)
EvidenceFS-->>daily_sentinel_operational_check_main: gsri_evidence
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: result_from_check("gsri_threshold", check_gsri(gsri_evidence, max_age_minutes))
daily_sentinel_operational_check_main->>EvidenceFS: load_json(worm_evidence_path)
EvidenceFS-->>daily_sentinel_operational_check_main: worm_evidence
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: result_from_check("pqc_worm_logger", check_worm(worm_evidence, max_age_minutes, worm_max_lag_seconds, expected_worm_bucket, require_compliance_object_lock))
daily_sentinel_operational_check_main->>EvidenceFS: load_json(attestation_evidence_path)
EvidenceFS-->>daily_sentinel_operational_check_main: attestation_evidence
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: result_from_check("tee_tpm_attestation", check_attestation(attestation_evidence, max_age_minutes))
alt json_output
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: render_json(results)
else markdown_output
daily_sentinel_operational_check_main->>daily_sentinel_operational_check_main: render_markdown(results)
end
daily_sentinel_operational_check_main->>Operator: exit(1 if any(result.is_failure) else 0)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
View changes in DiffLens |
|
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| Python | Jul 6, 2026 7:26p.m. | Review ↗ | |
| JavaScript | Jul 6, 2026 7:26p.m. | Review ↗ | |
| Shell | Jul 6, 2026 7:26p.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
📝 WalkthroughWalkthroughThis PR adds a new ChangesDaily Sentinel Operational Check
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Operator
participant CheckerCLI as daily_sentinel_operational_check
participant Dashboard
participant EvidenceFiles as Evidence JSON files
participant Report as Markdown/JSON output
Operator->>CheckerCLI: run with args (evidence paths, flags)
CheckerCLI->>Dashboard: HTTP HEAD/GET request (unless skipped)
Dashboard-->>CheckerCLI: status code or WARN if skipped
CheckerCLI->>EvidenceFiles: load G-SRI evidence JSON
CheckerCLI->>EvidenceFiles: load WORM evidence JSON
CheckerCLI->>EvidenceFiles: load TEE/TPM evidence JSON
CheckerCLI->>CheckerCLI: validate freshness, thresholds, signatures
CheckerCLI->>CheckerCLI: aggregate results into overall_status
CheckerCLI->>Report: render_markdown or render_json
CheckerCLI-->>Operator: print report, exit code (0 or 1)
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider validating CLI numeric options like
--max-age-minutesand--worm-max-lag-secondsup front (e.g., enforcing non-negative values) so misconfiguration fails fast with a clearer error message. - In
result_from_check, the explicitnameparameter is only used on exception paths while successful results rely on the inner check’sCheckResult.name; simplifying this helper to derive the name from the inner result would reduce the chance of name mismatches between the caller and the check implementation.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider validating CLI numeric options like `--max-age-minutes` and `--worm-max-lag-seconds` up front (e.g., enforcing non-negative values) so misconfiguration fails fast with a clearer error message.
- In `result_from_check`, the explicit `name` parameter is only used on exception paths while successful results rely on the inner check’s `CheckResult.name`; simplifying this helper to derive the name from the inner result would reduce the chance of name mismatches between the caller and the check implementation.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 2 medium |
| Documentation | 30 minor |
| ErrorProne | 2 medium 1 high |
| Security | 1 medium 47 high |
| CodeStyle | 8 minor |
| Complexity | 5 minor 1 critical 3 medium |
🟢 Metrics 104 complexity · 4 duplication
Metric Results Complexity 104 Duplication 4
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Blocking feedback
- Dashboard input validation can crash the CLI before any evidence report is emitted — scripts/daily_sentinel_operational_check.py#L129-L170
If you'd like me to push fixes, reply with item numbers (for example: please fix 1).
✅ Deploy Preview for onefinestarstuff canceled.
|
|
View changes in DiffLens |
|
View changes in DiffLens |
|
View changes in DiffLens |
|
View changes in DiffLens |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/test_daily_sentinel_operational_check.py (3)
109-113: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueUnescaped regex metacharacter in
match=.
match="pqc_worm_logger.py"treats.as "any character," not a literal dot. Ruff (RUF043) flags this. Currently harmless since the literal string also satisfies the pattern, but it's imprecise.🔧 Proposed fix
+import re + def test_check_worm_rejects_wrong_logger_name() -> None: evidence = valid_worm() evidence["logger_name"] = "other.py" - with pytest.raises(checker.CheckError, match="pqc_worm_logger.py"): + with pytest.raises(checker.CheckError, match=re.escape("pqc_worm_logger.py")): checker.check_worm(evidence, max_age_minutes=30, max_lag_seconds=900)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_daily_sentinel_operational_check.py` around lines 109 - 113, The `test_check_worm_rejects_wrong_logger_name` assertion uses `pytest.raises(..., match=...)` with an unescaped regex metacharacter, so the pattern is imprecise. Update the `match` argument in this test to use a properly escaped literal for `pqc_worm_logger.py` (or otherwise quote the pattern) so Ruff RUF043 is satisfied while still matching the `checker.CheckError` raised by `checker.check_worm`.Source: Linters/SAST tools
186-234: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWeak assertions only check for field-name presence, not failure state.
assert "gsri_threshold" in output(Line 208) andassert "gsri_threshold" in output(Line 232) only confirm the check ran; they don't confirm it actually reports FAIL/error. A regression that silently turns these intoPASSentries would still satisfy the assertion. Consider asserting on the status marker too (e.g.,"gsri_threshold | FAIL"or similar), consistent with how Lines 233-234 already assert"pqc_worm_logger | PASS".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_daily_sentinel_operational_check.py` around lines 186 - 234, The test assertions in test_main_returns_nonzero_when_evidence_is_stale and test_main_continues_after_individual_evidence_error are too weak because they only check for the gsri_threshold name; update them to assert the emitted status shows a failure state, matching the existing PASS checks for pqc_worm_logger and tee_tpm_attestation. Use the checker.main output captured by capsys to verify gsri_threshold is reported with a FAIL/error marker rather than just present in the text.
1-260: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo direct test coverage for
check_dashboard.Every
main()-based test passes--skip-dashboard, so the dashboard reachability check (HTTP HEAD/GET fallback, TLS options) added in this cohort has no unit test exercising it directly in this file. Consider adding at least one test (e.g., viaresponses/requests-mockor a local HTTP server) covering the HEAD/GET fallback path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_daily_sentinel_operational_check.py` around lines 1 - 260, Add direct unit coverage for check_dashboard in this test module, since all existing main() cases use --skip-dashboard and never exercise the dashboard reachability logic. Create at least one test around scripts.daily_sentinel_operational_check.check_dashboard that verifies the HEAD-to-GET fallback path, and if applicable another that covers TLS-related request options, using a mock HTTP layer or local test server so the behavior is validated without hitting a real endpoint.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/reports/DAILY_SENTINEL_V24_DEVSECOPS_RUNBOOK.md`:
- Around line 86-91: Remove the maintenance-window exception from the RED-state
criteria in the runbook unless there is an actual enforcement path in the
checker; the current CLI only exposes `--skip-dashboard`, so update the guidance
in the affected section to match what the checker can truly evaluate and keep
the remaining Sentinel health, G-SRI, WORM, and TEE/TPM conditions unchanged.
In `@scripts/daily_sentinel_operational_check.py`:
- Around line 106-127: The dashboard URL handling in _open_dashboard currently
passes any scheme through to urllib.request.urlopen, which can allow non-HTTP
resources. Add an explicit allowlist check for only http and https schemes
before the urlopen call, and make invalid schemes return a failed dashboard
check result with a clear error instead of raising or opening the resource.
---
Nitpick comments:
In `@tests/test_daily_sentinel_operational_check.py`:
- Around line 109-113: The `test_check_worm_rejects_wrong_logger_name` assertion
uses `pytest.raises(..., match=...)` with an unescaped regex metacharacter, so
the pattern is imprecise. Update the `match` argument in this test to use a
properly escaped literal for `pqc_worm_logger.py` (or otherwise quote the
pattern) so Ruff RUF043 is satisfied while still matching the
`checker.CheckError` raised by `checker.check_worm`.
- Around line 186-234: The test assertions in
test_main_returns_nonzero_when_evidence_is_stale and
test_main_continues_after_individual_evidence_error are too weak because they
only check for the gsri_threshold name; update them to assert the emitted status
shows a failure state, matching the existing PASS checks for pqc_worm_logger and
tee_tpm_attestation. Use the checker.main output captured by capsys to verify
gsri_threshold is reported with a FAIL/error marker rather than just present in
the text.
- Around line 1-260: Add direct unit coverage for check_dashboard in this test
module, since all existing main() cases use --skip-dashboard and never exercise
the dashboard reachability logic. Create at least one test around
scripts.daily_sentinel_operational_check.check_dashboard that verifies the
HEAD-to-GET fallback path, and if applicable another that covers TLS-related
request options, using a mock HTTP layer or local test server so the behavior is
validated without hitting a real endpoint.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99095d9a-ec16-4652-969b-4aa14078f5fb
📒 Files selected for processing (4)
docs/reports/DAILY_SENTINEL_V24_DEVSECOPS_RUNBOOK.mdpyproject.tomlscripts/daily_sentinel_operational_check.pytests/test_daily_sentinel_operational_check.py
Motivation
Description
scripts/daily_sentinel_operational_check.pythat performs checks for dashboard reachability,gsrithreshold/signature,pqc_worm_loggerS3 Object Lock commits, and TEE/TPM attestation, and emits Markdown or JSON output and a suitable exit code.daily-sentinel-operational-checkentry topyproject.tomlunder[project.scripts]for easy invocation as a project script.docs/reports/DAILY_SENTINEL_V24_DEVSECOPS_RUNBOOK.mddescribing required checks, example evidence JSON payloads, CLI usage, and fail-closed rules.tests/test_daily_sentinel_operational_check.pyto validate individual check logic, CLI behavior (offline and JSON modes), and error handling.Testing
tests/test_daily_sentinel_operational_check.pycoveringcheck_gsri,check_worm,check_attestation, timestamp parsing, andmainbehavior in offline and JSON modes.pytest tests/test_daily_sentinel_operational_check.pyand all tests completed successfully.Codex Task
Summary by Sourcery
Introduce a daily Sentinel AI v2.4 operational check CLI that validates key evidence planes and exposes them via a project script.
New Features:
daily-sentinel-operational-checkproject script for easy invocation.Documentation:
Tests:
Summary by CodeRabbit
New Features
Bug Fixes