W6/technical tails - #235
Merged
Merged
Conversation
T3 in STANDOFF-KEY asked for an empty scope to be a violation rather than a warning when --strict-scope is set. The flag did not exist: a grep across cyberai/ for strict_scope returned nothing, so the item as written could not be done. It is introduced here. The validator warned on an absent scope and let the run proceed. That is defensible as a default -- the pipeline has always behaved that way and a flag that changes behaviour for people who did not set it is a surprise, not a safety feature -- and indefensible as the only option. Absence of authorisation is not authorisation, and an engagement where that distinction matters had no way to say so. Under strict the phase now refuses: the orchestrator already raises on a failed verdict, so a violation is a failed exploit phase rather than a line of yellow text nobody reads. The switch travels as a parameter, not as an environment read inside the validator. A function that consults os.environ cannot be driven from a test without patching the process, and this one is otherwise pure. All eighteen existing call sites pass no strict argument and every one still gets a warning; the default is unchanged by construction and by test. One gap was found by mutation rather than by review. Deleting `strict=self.config.strict_scope` from _run_exploit left six new tests green: they drive the validator directly and the CLI directly, and neither can see that the phase in between stopped passing the value on. A flag that reaches the config and dies there is a producer without a consumer wearing full test coverage. The seventh test drives the phase itself, and both mutations -- dropping the argument, and neutering the raise -- now fail it.
T4 asked for the bare `except Exception` around a phase to be replaced with typed handling of network, LLM and tool errors. Measured against the code that is the wrong fix, and doing it would have made the pipeline worse. The clause wraps _dispatch, which reaches nmap, httpx, subprocesses, three LLM providers and several external binaries; no honest list of what they raise can be written, and a narrower clause trades a failed phase for a failed run. The comment calling it deliberate is correct and it stays. The real defect is the opposite one, and it was found by running the code rather than by reading it. Two exceptions were being caught here that are not phase failures at all: BudgetExceeded says a spending cap was crossed. Measured before the change: a run whose first phase hit the cap recorded a failed phase and moved to the next one, which was free to spend again. The cap stopped being a cap exactly when it began to matter. EgressViolation says the air-gapped path was asked to reach a remote provider. The run continued after the property it exists for had already been broken. Both now re-raise, in the sync path and in AsyncOrchestrator's copy of the same clause. The phase is still recorded as failed before the exception leaves, so the session says where the run ended rather than simply stopping. Everything else is still caught, and a test pins that: a dead target must not become a dead run. Recorded phase errors also carry their type now. "refused" alone reads the same whether a target dropped the connection or a defect raised it; the session export, the critic deciding on a retry, and whoever opens session.json get "ConnectionError: refused" instead. The format matches what mcp/client_probe.py already writes. Four mutations, four predicted victims: dropping either raise, emptying the fatal tuple, and reducing the description to str(exc). Dropping the sync raise leaves the async test green and vice versa, which is what proves the two copies are covered separately rather than by accident.
T2 asked for the intel agent to stop losing ports from the KB during CVE lookup. Measured, that does not happen: after the mass-open early return the port list is still in recon.nmap, untouched, and the skip is a deliberate trade the docstring explains -- a proxy answering every probe makes the service data noise, and spraying NVD with it returns garbage. The defect next to it is real. TLSCVEMapper had no product caller until a previous fix gave it one, and that fix wrote the enriched list to `intel.tls_findings`. Nothing reads that key: a grep across cyberai/ returns the write and nothing else, and the report renders session.findings, not the KB. Recon publishes no TLS finding either. So on a real target an expired certificate, a negotiated TLS 1.0 and RC4 in the cipher suite were probed, classified, matched to CVEs -- and then absent from the document the client reads. The cure reproduced the disease one level up. Each enriched TLS problem now becomes a finding. Severity travels from the TLS classifier rather than being decided again here: it already knows a deprecated protocol outranks a weak cipher. An unrecognised level degrades to INFO rather than raising, because losing a finding over a spelling is worse than under-rating one. A condition with no matching CVE still reaches the report and says so -- unmapped is a configuration problem, not the absence of one. The call keeps its position above both early returns, and a test now covers the mass-open path as well as the no-ports one. Those are exactly the runs a TLS-only target lands in, and a fix that only worked on the full path would miss both. tests/unit/test_intel_tls_context.py stays green through every mutation of the new code, which is the point: it pins the producer, and the new file pins the consumer. Removing the finding loop reddens five of six new tests and none of the four old ones.
T1 asked for authorized_scope to reach the exploit agent. It already does -- _scope_ok has been reading session.authorized_scope since before this sprint. What the item missed is that the guard reached one caller out of three. Its own docstring promised nuclei and the out-of-band path. Measured: _scope_ok returned False for a target the scope excluded while _oob_confirmer built a collector for that same target and handed it to the walk. The HTTP walk itself never asked at all -- it sent the full payload corpus at whatever host it was given. The orchestrator gates session.target, which is the same value on a normal run and is not the value these methods receive, so a direct call bypassed authorisation entirely. That bypass is what the in-agent guard was added to close. Both now consult it, and the OOB check sits above the collector probe: an unauthorised host must not even cost a request to phantom-grid. An empty scope is still a no-op, which is what the bench profile relies on. One test in this file was worthless and mutation found it. The first version of the web-path test asserted zeros against an agent with an empty knowledge base, so it read the "no HTTP surface" branch and called it a refusal; with the guard deleted it stayed green. It now puts a surface in the KB and patches exploit_surface, so a guard that fails to fire shows up as a call. Also: Development Status classifier moves from Alpha to Beta. It said Alpha while the package shipped a wired trust boundary, decontaminated proofs, an architecture-tested README and 2280 tests. Beta claims the interfaces are stable enough to build against and that the failure modes are known and written down -- not that the work is done.
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
2 participants
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.
What this changes
How it was measured
Checklist
ruff format --check cyberai/ tests/andruff check cyberai/ tests/passpytest -W ignore::DeprecationWarning -m "not slow and not smoke"passes