Repository navigation
Conversation
Phase 10, step 5 plan, PR 7. The security layer's evidence log must
hold every step of an engagement. A layer registers a recorder with
add_step_recorder(); the executor tells it about each step before it
runs ("start"), after it ends ("done", with its result, also when the
tool crashed or returned a malformed result) and when the safety engine
refuses it ("refused"). A step whose start cannot be recorded does not
run, so nothing runs unrecorded; a failure to record the end is added
to the step's error output.
* src/codebud/safety.py (_STEP_RECORDERS, add_step_recorder)
(remove_step_recorder, record_step): New.
* src/codebud/executor.py (Executor.execute_plan): Record each step.
* tests/test_step_checks.py: Test them.
* docs/chapters/safety-and-sandbox.texi (The Safety Engine): Document
them.
Phase 10, step 5 plan, PR 7 (D14a). codebud-sec chat|run|plan --engagement FILE runs codebud inside an engagement: it loads the engagement, opens its evidence log (created encrypted beside the file, <name>.evidence.jsonl, or the one --evidence names; reopened with its chain checked for later sessions), activates the scope guard and network fence, registers the evidence recorder, and hands over to codebud with a security profile -- security-pentest unless --profile names another; other options pass through. When codebud ends, everything is taken down and the session's end and exit status are recorded. Every step goes into the log before it runs and after it ends, refused steps as warnings; text over 64 KiB is kept as its SHA-256, size and head. codebud-sec evidence add records files with their SHA-256 and a note; verify checks the chain (--files: the recorded files too) and prints the head hash to keep elsewhere; show lists the entries; close ends the log. codebud-sec pin is codebud integrity pin. Verified live with Ollama: a planned nmap step without --unprivileged was refused at plan time; with it, codebud-sec run ran it behind the fence, and the log held both records and verified. The layer now calls codebud's own main: CORE_API += codebud.cli.main. make install and pip install both provide the command. * src/codebud_sec/cli.py, src/codebud_sec/__main__.py: New files. * pyproject.toml (project.scripts): Add codebud-sec. * Makefile.am (bin_SCRIPTS): Build codebud-sec from the launcher. * src/launcher.in: Name the module and command. * src/Makefile.am: Install the new modules. * .gitignore: Ignore the built launcher. * tests/test_layers.py (CORE_API): Add codebud.cli.main. * tests/test_codebud_sec.py: New file. * docs/chapters/security.texi: New chapter, "Security Work". * docs/codebud.texi, docs/Makefile.am: Include it. * docs/chapters/architecture.texi, docs/chapters/development.texi: Mention them. * README.md, NEWS: Describe codebud-sec.
A step the engagement's scope refused was explained as a safety policy refusal, whose hint is to add the command to an allow-list -- which cannot widen an engagement's scope. It now says the step reaches outside the engagement or cannot be read, and to rephrase it so each step names only in-scope targets and meets the reason given. * src/codebud/cli/errors.py (_RULES): Add "engagement scope" before "safety policy". * tests/test_codebud_sec.py (test_a_scope_refusal_gets_its_own_hint): New test.
Owner
Author
Review summary: changes requestedOne full sweep over Conditions.
The invariants behind them:
Correct, verified (1796 tests, plus live runs with Ollama):
Defects (each reproduced on
Verdict: changes requested. The fixes come as new commits, then a re-review. |
…engagement Review of #23. Three defects, each reproduced: The default log lay beside the engagement file, usually in the project, which steps may write (file_write accepted it; any command step could write it too): a step could truncate it or replace it consistently. It now lives in codebud's data directory, evidence/<engagement>-<id>.jsonl, which steps never see; an --evidence path inside the project is refused. The engagement file was reloaded at each session without comparing it to the SHA-256 the log's open entry records: a scope widened to 0.0.0.0/0 between sessions opened on the same log. Reopening a log now unlocks the key, checks the whole chain, and refuses an engagement file other than the one the log was opened with. A message codebud exited with (sys.exit("...")) became status 1 with no text; it is printed now, as Python would. CORE_API grows by codebud.paths (the data directory) and codebud.confine (the project root), deliberately. * src/codebud_sec/cli.py (evidence_path): The data directory by default; refuse a path inside the project. (_open_log): Unlock, check the chain and the engagement file. (_session): Show codebud's exit message. (_log_for): Report a refused path. * tests/test_layers.py (CORE_API): Add codebud.paths, codebud.confine. * tests/test_codebud_sec.py: Follow the new place; test the fixes. * docs/chapters/security.texi (Running codebud-sec), NEWS: Say where the log is kept and that a changed scope needs a new log.
Owner
Author
Re-review summary: fixed, ready to merge once CI passesSecond sweep, over
This sweep:
1802 tests pass, |
This branch has not been deployed
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.
Phase 10, step 5 plan, PR 7 (D14a): the
codebud-seccommand, the evidence log wired to every step, and a manual chapter, "Security Work".Commits
c61aa3cStep recorders (core). Withadd_step_recorder(), the executor tells a recorder about every step: before it runs (start), after it ends (done, also when the tool crashed or returned a malformed result), and when it is refused (refused). A step whose start can't be recorded doesn't run, so nothing runs unrecorded.02c4a42Thecodebud-seccommand.codebud-sec chat|run|plan --engagement FILEloads the engagement and opens its evidence log: created encrypted beside the file, or reopened with its chain checked. Then it activates the scope guard, the network fence and the evidence recorder, and hands over to codebud withsecurity-pentest, unless--profilenames another. Other options pass through.evidence addrecords files with their SHA-256 and a note.verify [--files]checks the chain and prints the head hash;showlists the entries;closeends the log.piniscodebud integrity pin.CORE_API += codebud.cli.main, a deliberate widening.make install(a second launcher) and pip (project.scripts) install the command.165b098Its own hint for scope refusals. They used to get the allow-list advice, and no allow-list can widen an engagement's scope.Verification
make check: 1796 passed.make distcheckinstalls and uninstalls both commands. The info and HTML manuals build without warnings; ruff is clean.codebud-sec pinpinned 2 models and 71 binaries, pasta, nft and sh among them.nmapstep without--unprivilegedwas refused at plan time, with the fence's reason.--unprivileged,codebud-sec runran it behind the fence and it found the router's ports. The log held the step's start and done records, andevidence verifyreported the chain intact.