Repository navigation
Fix review findings #105/#106/#107 (12.1.11) - #108
Merged
Merged
Conversation
…al capture (#105) - core/cleanup.py: install signal handlers on the running loop instead of run_until_complete(), which raised on the live TUI loop and skipped interface and iptables teardown on Ctrl-C. - core/audit.py: treat an external append as a segment boundary so verify() no longer goes permanently false after a concurrent writer; verify_file() stays the authority for the on-disk chain. - automation/handlers/setup.py: substitute the loot dir into the generated capture.php (was a literal placeholder in a non-f-string; captures were lost).
… gate (#106) - chaining/executor.py: implement condition (pre-run skip), on_error RETRY (retry_count/retry_delay), FALLBACK (fallback_tool) and SKIP; defaults (STOP, no condition) keep the previous behaviour. Uses paths.resolve_path for the condition value, so paths.py is now a live module (reachability ledger updated). - sessions/manager.py: optimistic-concurrency update() (WHERE id AND updated_at=baseline, re-read and retry on a miss) so racing callers no longer silently clobber each other; no schema change. - safety: check_target_safety removed from the package public API and documented as not an authorisation gate, so it cannot be mistaken for the scope gate. - Not addressed here: the tool_requirements preflight registry is still consumed only in the TUI path (M1); wiring it into non-TUI execution is a design call. Tests: test_chain_error_policy.py, test_session_optimistic_update.py.
…chor note (#107) - keys.py: create the API key file with mode 0o600 at os.open time instead of write_text + chmod, closing the world/group-readable window. - automation/handlers/cleanup.py: restore_managed_mode and flush_iptables now check each command's result instead of always returning True; removed the stray 'completed' debug log in the failure branch. - Removed duplicate back-to-back log calls (control.py, tools/base.py). - SECURITY.md: note that audit truncation detection fails open when the .anchor sidecar is missing.
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
1 participant
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.
Fixes from the read-only review, verified with the repo's own ruff, mypy and pytest on the touched paths. Bumps to 12.1.11.
#105 (high)
core/cleanup.py: install handlers on the running loop (add_signal_handler) instead ofrun_until_complete(), which raised on the live TUI loop and skipped interface/iptables teardown on Ctrl-C. The synchronous fallback never callsrun_until_completeon a running loop.verify()core/audit.py: an external append is now a segment boundary, soverify()no longer goes permanently false after a concurrent writer.verify_file()stays the authority for the on-disk chain.automation/handlers/setup.py: the loot dir is substituted into the generatedcapture.php(was a literal placeholder in a non-f-string; every capture was written to a dead path).#106 (medium)
chaining/executor.py:condition,RETRY/retry_count/retry_delay,FALLBACK/fallback_toolandSKIPare now honoured. Defaults (STOP, no condition) preserve the old behaviour, so existing chains are unaffected.sessions/manager.py: optimistic concurrency (WHERE id AND updated_at = <baseline>, re-read and retry on a miss). No schema change.safety/:check_target_safetyremoved from the public API and documented as not an authorisation gate.tool_requirementspreflight registry is still consumed only in the TUI path (finding M1). Wiring it into non-TUI execution is a design decision, so Medium: chain retry/fallback ignored, session lost-update, mislabelled safety primitive, TUI-only preflight #106 stays open for it; see the issue comment.#107 (low / hygiene)
0o600atos.opentime (waswrite_textthenchmod).restore_managed_mode/flush_iptablesreport command failures instead of always returningTrue; stray "completed" debug log removed from the failure branch; duplicate log calls removed.SECURITY.md: note that audit truncation detection fails open without the.anchorsidecar. Cleared stale.coverage/.hang-diagnostic.log.Verification
test_audit_concurrent_interleave.py,test_chain_error_policy.py,test_session_optimistic_update.py.mainon the changed files.Closes #105
Closes #107