[MBK-fr]

Adds a Darkmoon tool to the Atomic Forge so an Atomic Agents agent can run an autonomous pentest and read campaigns and findings from a self-hosted Darkmoon (https://github.com/ASCIT31/Dark-Moon, GPL-3.0 autonomous AI penetration testing platform).

atomic-forge/tools/darkmoon_pentest follows the existing forge layout (tool/, tests/, README.md, pyproject.toml, requirements.txt, .coveragerc) and the Serply Search structure: DarkmoonPentestTool(BaseTool[...]) with DarkmoonPentestToolInputSchema / OutputSchema / Config, aiohttp based run_async plus a sync run. One action field selects run_pentest, list_campaigns or get_findings.

It calls the Dashboard API of an instance the user operates (DARKMOON_BASE_URL / DARKMOON_USERNAME / DARKMOON_PASSWORD or config fields): login at /api/v1/auth/login, then /api/v1/run/campaign, /api/v1/run/logs/{run_id}, /api/v1/campaigns, /api/v1/vulnerabilities. There is no public hosted endpoint. Open source vs Pro is stated in the README: the engine and CLI are open source, the dashboard API this tool uses is part of Darkmoon Pro.

Design notes: the run action only reports a campaign created by that run (never a pre-existing one), reports timed_out instead of waiting forever, validates required fields per action, and surfaces the API detail message without echoing credentials.

Also registers the tool in AGENTS.md, atomic-forge/README.md, README.md, docs/guides/tools.md and adds the workspace member to uv.lock (uv lock --check passes).

Tests: 13 tests against an in-process aiohttp server that mimics the dashboard routes (login, campaigns, vulnerabilities, run, run logs): schema validation, env fallback, bad credentials, list, findings, run with wait, run without wait, timeout, stale campaign guard, sync wrapper. pytest atomic-forge/tools/darkmoon_pentest/tests passes, black --check and flake8 clean per the CI workflow.

🤖 Generated with Claude Code

https://claude.ai/code/session_014HBQNzAf5C2E3MiJC48HQw

[Eigenwise]

Historical review of 7f0dd251.

This needs changes before merge. Network-blocked mocked probes reproduced:

• tool/darkmoon_pentest.py:219/:243-247: a fresh campaign from another concurrent run is attached to the current target, returning the other target's findings. A created campaign must be correlated to its run and target, not selected because it appeared recently.
• :14/:205-206/:242: run_error returns a normal zero-findings result. Failed assessments must remain explicit failures.
• :194-209: run_timeout=1 with poll_interval=100 waits 100 simulated seconds. Cap both polling sleeps and requests by the remaining deadline.
• :199-203: an HTTP 500 whose detail contains "404" is swallowed. Use the actual HTTP status.
• :174-186: malformed success envelopes become empty campaigns or zero findings. Validate the API shape instead of presenting incompatibility as a clean result.

The config also exposes its password in repr, and the schema accepts CVSS=99. Please add regressions for these cases and secret/TLS/cancellation boundaries. Eight production functions measured complexity at least 6 on this historical head.

The Pro-only dashboard dependency is disclosed. Please pin the supported Pro API/version and provide contract fixtures; the local imitation server alone does not establish compatibility. I did not run a pentest, contact an instance, or use credentials.

The later 00611572 review ran 64 revised tests and confirmed the failed-run, deadline, HTTP-status, CVSS and repr repairs. At 4fa08965, the completed source review confirms status-only run output with explicit campaign selection, a shared redirect deadline and null-stats validation. The old automatic-attribution finding is superseded. The two remaining diagnostics were source-derived at that recheck. Eight later offline reproduction/control cases on 4fa08965 confirmed both. Full-suite testing and private API compatibility remain unverified.

[MBK-fr]

Thanks for the detailed review. Disclosure: I contribute to Darkmoon (the tool's upstream project). Pushed 0061157 (plus a merge of current main), point by point:

• Run/campaign correlation: POST /run/campaign returns only run_id (and pid, command), so there is no campaign id to read from it. The tool now takes the first sessionID in the run log and selects the campaign whose 8-char session_id is contained in it, which is the rule the Dashboard API itself uses to finalise a run's campaign. The newest-campaign heuristic and the pre-run snapshot are gone. Zero or more than one match, or no session id in the log, raises DarkmoonCorrelationError. I did not add a separate target-equality check, because the campaign record has no reliable target field to compare.
• run_error: run_error, an aborted run, or a non-zero exit code raises DarkmoonRunError (message includes the run id, error text truncated).
• Deadline: each poll request timeout and each sleep are capped by the time left before run_timeout. Tests: run_timeout=1, poll_interval=100 returns in about 1s; a slow log endpoint is cut at the deadline; a slow request far from the deadline is still an error.
• HTTP status: DarkmoonAPIError carries .status; only a real 404 on the run log means "not written yet". A 500 whose detail contains "404" now raises.
• Response shape: login, run start, run logs, campaigns and vulnerabilities are validated; malformed envelopes, untitled findings, bad total/stats raise DarkmoonSchemaError instead of becoming empty results.
• Secrets/CVSS/TLS/cancel: password is repr=False (tests cover config, tool, str, and error messages); cvss_score is constrained to 0-10; base URL must be http(s); a self-signed certificate is rejected (test uses a throwaway cert); task cancellation propagates and stops polling.
• API pin and fixtures: README pins Dashboard API major 1 (app version 1.0.0, /system/info contract_version "1", Pro v1.4.x). The tool reads /api/v1/system/info and refuses other majors (an instance without that route is accepted as unversioned). tests/fixtures/ holds contract bodies for login, system info, run start, run logs (completed/error/aborted), campaigns and vulnerabilities. Caveat: they are written from the Dashboard API route source and the published @darkmoon_ai/client 0.2.0, not recorded from a live instance, and the README says so.
• Complexity: every function (production and tests) is now below 6 by radon; flake8 at --max-complexity=5 is clean.

Checks run locally: 64 tests pass with 100% line coverage of tool/, black and flake8 (repo config) clean, mypy clean on the tool.

[Eigenwise]

Historical review of 00611572.

Rechecked 00611572 offline. The 64 revised tests pass, including failed-run, deadline, HTTP-status, CVSS, repr, TLS and cancellation cases. All 95 separately measured functions now have native CRAP scores below 6. The links are neutral and affiliation is disclosed.

Remaining findings in tool/darkmoon_pentest.py:

• Login credential forwarding, line293: a 307 redirects the login POST to another origin and forwards the username/password body. Two loopback listeners reproduced this with dummy credentials. Reject cross-origin credential redirects.
• Secrets in diagnostics, lines154/226: API and run error text echoes the complete dummy password/token. Protect exception/log messages as well as repr.
• Malformed findings, lines207-210: total=-9 and stats.by_severity='broken' are accepted and produce normal output. Validate the documented counts/stats shape.
• Version contract, lines238-240: malformed HTTP200 system info, garbage api_version, and contract_version=2 are accepted. The documented compatibility fallback is for an absent route; a present invalid or incompatible contract needs rejection.

Campaign correlation still needs authoritative evidence. Lines160/379 accept an arbitrary eight-character substring as the sole campaign match. Missing and ambiguous matches are rejected, but the pinned public upstream lacks the claimed Pro route source, so the source-derived fixtures don't independently establish safe concurrent attribution. Please provide a verifiable ID-mapping contract and tests tied to it. I have not claimed a live concurrency reproduction.

Black127 and canonical Flake8 pass. Production statement coverage is 255/255 with 15 existing example exclusions; branch coverage is unmeasured. No vendor installation, real dashboard login, real credentials, target requests or pentest execution occurred.

These findings are superseded by later source repairs: the f0476a1d review credits same-origin redirects, diagnostic scrubbing, counts/stats validation and version handling. The completed 4fa08965 source review confirms automatic campaign attribution was removed in favor of status-only output and explicit selection, plus the whole-request deadline and null-bucket repairs. The 64-test result and measurements above belong to 00611572. Eight later offline reproduction/control cases on 4fa08965 confirmed both remaining diagnostic findings. Full-suite testing and private API compatibility remain unverified.

[MBK-fr]

Thanks for the second pass. Pushed f0476a1 addressing all four blocking items plus the campaign-mapping evidence, point by point.

1. Cross-origin credential redirect (line ~293). _request now disables the client's automatic redirect following (allow_redirects=False) and resolves redirects by hand: a 3xx whose Location resolves to a different origin than base_url raises DarkmoonRedirectError and is never followed, so the login POST body and the bearer token can't be replayed to another host. Same-origin redirects (e.g. a trailing-slash 307) are still followed, bounded to MAX_REDIRECTS hops; a missing Location or a redirect loop also raises. Tests: test_cross_origin_login_redirect_is_refused_and_credentials_not_forwarded (two loopback listeners, asserts the second origin received no login), test_cross_origin_redirect_target_is_refused, test_same_origin_redirect_target_is_followed, test_same_origin_redirect_is_followed_end_to_end, test_redirect_without_location_is_refused, test_redirect_loop_is_bounded.

2. Secrets in diagnostics (lines ~154/226). The tool keeps a redaction set of its secrets (password from config/env, bearer token once /auth/login returns it) and scrubs it from every raised message: the API detail surfaced by DarkmoonAPIError and the run-error text in DarkmoonRunError both go through _scrub before the exception is built, so a verbose or hostile detail/error can't echo them back. (The tool emits no log records; exceptions are the only diagnostic surface.) Tests: test_api_error_text_is_scrubbed_of_password_and_token, test_run_error_text_is_scrubbed_of_token (plus the existing repr/str cases).

3. Malformed findings counts/stats (lines ~207-210). _parse_findings now requires total to be a real non-negative integer that is not below the number of findings returned, and each stats bucket (by_severity/by_category/by_status) to be an object mapping to non-negative integer counts. total=-9 and stats.by_severity='broken' (and negative/non-int bucket counts) now raise DarkmoonSchemaError. Tests: test_malformed_counts_or_stats_are_rejected (parametrized) and test_well_formed_counts_and_stats_are_accepted.

4. Version contract (lines ~238-240). A present /system/info (HTTP 200) must be a JSON object whose api_version major is 1 AND whose contract_version is "1". Garbage api_version, a missing field, a different major and contract_version=2 all fail closed before authenticating; the "accept as unversioned" fallback is now only for an absent route (HTTP 404). Tests: test_present_but_invalid_version_contract_is_refused (parametrized over the four cases you listed plus a non-object body) and test_supported_contract_version_is_accepted.

5. Verifiable campaign id-mapping (lines ~160/379). The mapping is now explicit and enforced, not a bare substring. Darkmoon creates a campaign id as camp_<YYYYMMDD>_<session_id> with session_id = uuid4().hex[:8] (api/live_push.py), so a conformant record has a session_id that is 8 lowercase hex chars AND is exactly the trailing segment of its id. A run is attributed to the one campaign whose verified token is an 8-char window of the run's normalised parent session id, i.e. the window rule the dashboard uses to finalise a campaign (api/live_push.py, api/routes_run.py), tightened so the window token must also be the token encoded in the id. A campaign whose id encodes a different token is no longer eligible even if its session_id happens to be a substring of the session, and zero/ambiguous matches still raise. Tests: test_campaign_session_token_contract, test_campaign_session_token_rejects_nonconformant (parametrized), test_owns_session_requires_window_and_id_consistency, and test_campaign_whose_id_does_not_encode_its_session_token_is_not_attributed.

On the evidence point: the fixtures are still written from the Dashboard API route source and reflect the id/session_id derivation above rather than being recorded from a live instance, and the README continues to say so. The id<->token invariant is what makes the attribution checkable offline; the window rule mirrors the server's own finalisation.

Checks run locally: 98 tests pass with 100% line coverage of tool/; black --check and flake8 (repo config, and --max-complexity=5) are clean; mypy clean on the tool.

[Eigenwise]

Historical review of f0476a1d.

Source-only recheck of f0476a1d: manual same-origin redirects, known-secret scrubbing, counts/stats validation and version handling are improved in the source. I haven't run this head's tests or security probes; the earlier 64-test result belongs to 00611572.

On this historical head, DarkmoonPentestTool._request at tool/darkmoon_pentest.py:381-404 had native Radon 6.0.1 complexity 6. The earlier score-only refactor request is withdrawn.

Campaign ownership also still needs independently verifiable contract evidence. The ID/token suffix checks are clearer, but accepting a matching eight-character window in the parent session ID doesn't establish concurrent run attribution. Source-derived fixtures alone leave that compatibility claim unverified.

Current-head runtime, coverage and style checks remain unverified. No vendor installation, dashboard login, real credentials, target requests or pentest execution occurred.

The later aef00856 source review measures the refactored request path below complexity 6. The completed 4fa08965 source review confirms automatic findings attribution was removed entirely: run returns status, and list/get require explicit selection. The complexity and automatic-attribution requests above are superseded. Eight later offline reproduction/control cases on 4fa08965 confirmed both remaining diagnostic findings. Full-suite coverage and private API compatibility remain unverified.

[MBK-fr]

Thanks for the third pass. Pushed aef0085 addressing both remaining points.

1. _request complexity (Radon 6.0.1). You were right that _request scored cyclomatic 6 (one for, two if, two header/timeout ternaries). I split the redirect/request flow at meaningful boundaries:

• _send_once() performs exactly one request with allow_redirects=False and returns (next_url, payload) — a same-origin redirect target still to follow, or None with the final body. CC 3 (A).
• _auth_headers() and _request_timeout() hold the two ternaries. CC 2 (A) each.
• _request() is now just the bounded loop that drives _send_once. CC 3 (A).

radon cc -s tool/darkmoon_pentest.py now reports every function at grade A; the previous top of the list is gone:

M 5 DarkmoonPentestTool._read_only_action - A (5)
M 5 DarkmoonPentestTool._run_body - A (5)
M 5 DarkmoonPentestTool._campaign_of_run - A (5)
M 5 DarkmoonPentestTool._check_settings - A (5)
M 5 DarkmoonPentestTool.__init__ - A (5)
F 5 _terminal_event / _failure_of / _first_session_id - A (5)
M 3 DarkmoonPentestTool._request - A (3)
M 3 DarkmoonPentestTool._send_once - A (3)

All 141 measured functions (production + tests) are < 6. Behaviour is unchanged: cross-origin credentialed redirects are still refused, same-origin followed and bounded to MAX_REDIRECTS, missing Location and loops still raise — the existing redirect tests pass untouched.

2. Campaign ownership — I took the honest-narrowing path (b), not a stronger claim. I went back to the Dashboard source rather than defend the window rule, and you are correct: there is no independently verifiable run→campaign key in the contract, so I have not claimed one.

• The campaign record (api/live_push.py) carries only id / project_id / target_id / session_id / is_subagent / status / stats / ... — no run_id, no owner, no full session id.
• Its session_id is the dashboard MCP server's SESSION_ID = uuid4().hex[:8] (mcp/src/server.py), generated independently of the opencode sessionID that appears in the run log — they are not derived from each other.
• POST /api/v1/run/campaign returns only {run_id, pid, command} (api/routes_run.py), and CampaignRunRequest has no field to inject a correlation id.
• The only linkage the server itself uses is _finalize_stale_campaigns (api/routes_run.py): a campaign's 8-char session token is matched as a substring of the run's parent session id, restricted to non-subagent campaigns — a best-effort reconciliation sweep, not unique attribution.

So the matching window is genuinely all the contract offers, and it cannot disambiguate truly concurrent runs. Instead of overstating it I narrowed the claim in code, docstrings and the README to say exactly what it does and does not guarantee, and made the tool conservative:

• _owns_session() now also excludes is_subagent campaigns (mirroring the server) on top of the existing id↔token invariant, which cuts false matches.
• The tool fails safe: zero or more than one eligible campaign raises DarkmoonCorrelationError rather than returning a possibly-wrong campaign's findings — fall back to list_campaigns + get_findings.
• Two new tests tie this to the behaviour you asked about: test_subagent_campaign_is_never_attributed, and test_concurrent_ambiguous_campaigns_refuse_rather_than_guess (two concurrent non-subagent campaigns whose tokens are both windows of the run's session → the tool refuses, it does not pick one). The fixtures still reflect the id/session_id derivation in the source and are not live recordings — I am not claiming a live concurrency reproduction.

In short: the verifiable part is the offline id↔token conformance and the fail-safe refusal; concurrent-run ownership is explicitly documented as not guaranteed, because the contract exposes no key for it.

Local checks on this head: radon cc -s all grade A (< 6); 101 tests pass with 100% line coverage of tool/ (pytest --cov=tool); black --check (line-length 127) and flake8 (repo config and --max-complexity=5) clean; mypy clean on the tool.

[Eigenwise]

Historical review of aef00856.

Source-only recheck of aef00856: the request path was refactored. Native Radon 6.0.1 measures the production functions at complexity 5 or below. Same-origin redirect checks and the added subagent/ambiguous-match refusals remain in the source. Current-head tests, coverage and style have not been independently verified.

The ownership narrowing still leaves a wrong-findings path at tool/darkmoon_pentest.py:553-559,582-585. A new completed run with session ses_2a9e5f3a9c1eAbCdEf0123 accepts the sole old campaign camp_20200101_5f3a9c1e, token 5f3a9c1e, is_subagent=false, and pairs that campaign's findings with the new run ID. The suffix/window checks don't establish ownership. This is a source-derived counterexample, not a live reproduction. Please stop automatic findings attribution when the API supplies no verifiable run-to-campaign key. Returning the run status and requiring explicit campaign selection would preserve the existing list/get actions. Update the remaining return its findings and fails safe promises too.

Three further source-backed gaps:

• :491,420-423,523-524: each redirect hop reuses the original remaining timeout, and a terminal result is accepted without rechecking the deadline. Two 0.75-second hops can exceed a one-second run budget. Use the remaining whole-run budget for every hop and final result.
• :339,275-276: version values and stats bucket names bypass secret scrubbing when interpolated into errors. Route these messages through the same diagnostic protection.
• :283-285: an explicitly null stats bucket skips validation. Reject stats.by_severity=null when a bucket object is required.

The source-derived Pro fixtures still don't independently establish the private API contract. No vendor code installation, dashboard login, real credentials, target requests or pentest execution occurred. Useful tests should stay; current-head coverage was not measured.

The completed source review of 4fa08965 supersedes these attribution, redirect-budget and null-bucket requests: it confirms status-only output with explicit campaign selection, one redirect deadline with a final-body check, and rejection of null buckets. Plain version/stat diagnostics are also repaired. Eight later offline reproduction/control cases on 4fa08965 confirmed the two remaining escaped-version and timeout-path gaps. Full-suite testing and private API compatibility remain unverified.

[MBK-fr]

Thanks for the fourth pass, and specifically for the ses_2a9e5f3a9c1eAbCdEf0123 / camp_20200101_5f3a9c1e counterexample. It's correct: an 8-char window of a run's session id equalling an old campaign's session_id does not establish ownership, so the tool should not have attributed it. Pushed 4fa0896, point by point.

1. No more findings auto-attribution. You're right that the contract gives no verifiable run→campaign key, so I removed the attribution instead of keeping a heuristic that claims ownership we can't verify. Confirmed against the source: the campaign record (mcp/api/live_push.py, the campaign = {...} dict) carries only id / project_id / target_id / session_id / is_subagent / date / status / stats / ... — no run_id, no owner, no full session id — and session_id is the dashboard MCP server's independent SESSION_ID = uuid4().hex[:8] (mcp/src/server.py). run_pentest now returns the run status (run_status: started / completed / timed_out) and attributes nothing; findings are read explicitly with list_campaigns + get_findings for a chosen campaign id (both actions preserved). A failed run still raises DarkmoonRunError. The session-window/suffix correlation code, DarkmoonCorrelationError, and the returns its findings / fails safe wording in the docstrings and README are gone. Tests: test_completed_run_does_not_attribute_a_window_matching_old_campaign encodes your exact counterexample (sole camp_20200101_5f3a9c1e, is_subagent=false, run session ses_2a9e5f3a9c1eAbCdEf0123) and asserts the run returns completed with campaign_id is None, no findings, and that neither the campaign list nor vulnerabilities are fetched; test_explicit_campaign_selection_still_returns_findings proves the list/get path still reads findings by explicit id; test_run_pentest_sends_body_and_reports_status_without_attributing_findings covers the body + status contract.

2. Redirect timeout budget. _request now derives one whole-request deadline (_deadline) and spends it across every hop: each hop gets only deadline - now, a hop that would start past the deadline is refused, and the deadline is rechecked before a terminal body is accepted — so two slow hops can't exceed the budget. Tests: test_redirect_hops_share_one_whole_request_budget (two same-origin 0.4s hops under a 0.5s budget → times out at ~0.5s, not ~0.8s), test_terminal_result_past_the_deadline_is_rejected (terminal body returned after the deadline is rejected), test_redirect_hop_that_consumes_the_budget_stops_before_the_next_hop (no further hop once the budget is spent). The existing same-origin / loop / missing-Location redirect tests still pass.

3. Secret scrubbing for version values and bucket names. _check_api_version and the stats-bucket validation now route their messages (which interpolate API-supplied api_version / contract_version values and stats.<bucket>.<name> keys) through the same _scrub used for API and run-error text. Tests: test_version_value_is_scrubbed_in_error (a /system/info whose api_version embeds the password) and test_stats_bucket_name_is_scrubbed_in_error (a by_severity key embedding the password with a bad count) both assert the secret is replaced with ***. As you noted, exception-chain/local exposure is out of scope here.

4. Null stats bucket. _require_stats now checks bucket presence (key in stats) rather than truthiness, so a present stats.by_severity = null is validated and rejected as a non-object; a genuinely absent bucket stays optional. Tests: test_null_stats_bucket_is_rejected (parametrized over all three buckets) and test_absent_stats_bucket_is_still_optional.

Checks on this head: radon cc -s reports every one of the 133 measured production/test blocks at grade A (max 5, < 6); pytest --cov=tool is 94 passing with 100% line and branch coverage of tool/; black --check (line-length 127), flake8 (repo config and --max-complexity=5) and mypy (--ignore-missing-imports) are clean.

On evidence: the fixtures remain written from the Dashboard source and the README continues to say they are not live recordings. With auto-attribution removed, they only pin response shapes; none of the four changes now relies on a concurrent-attribution claim.

[Eigenwise]

Checked 4fa089656fe2dfc2aa9e0d6c3a0daae785039e59 with eight offline reproduction/control cases using dummy credentials, mocked HTTP responses and a deterministic clock. Both diagnostic findings reproduce. No engine, backend, login, target or remote requests were used.

• Version messages, lines293/300: with configured dummy password abc\def and an identical returned api_version or contract_version, the actual DarkmoonSchemaError contains the escaped representation 'abc\\def'. Decoding that literal recovers the full password. Ordinary version controls still display correctly. Please redact the value before repr formatting and cover both fields with a backslash-containing password.
• Timeout messages, lines390/395: a mocked run-start response returns configured dummy password s3cret-Passw0rd as its run ID. Both a late response and an exhausted request budget propagate TimeoutError: Darkmoon API request to /api/v1/run/logs/s3cret-Passw0rd exceeded its time budget. through the polling caller. Non-secret run-ID controls produce the expected timeout too. Please scrub the path in both messages and add polling-path regressions.

All eight reproduction/control checks passed, confirming these defects and the ordinary controls. Thanks for the status-only run output, shared redirect deadline and null-bucket fixes. This focused check doesn't establish full-suite coverage or private API compatibility. I'm withdrawing the earlier vague Any/naming request and untested exception-chain/local concerns from this closeout.

[Eigenwise]

Sorry for the repeated review rounds. We should have consolidated our findings and finished our own checks before asking you for more changes.

Your fixes on 4fa08965 are acknowledged: status-only run output, the shared redirect deadline, and null-stats validation.

The focused eight-case offline check has now confirmed the two remaining diagnostic-redaction defects, using dummy credentials and mocked responses. The consolidated review has the reproductions and the two specific requests: redact version values before repr formatting, and protect timeout paths through polling. No remote requests were made. Full-suite quality and private API compatibility remain unverified.

The vague "Any/naming gaps" request is withdrawn. The untested exception-chain/local-exposure concern remains outside this closeout. This comment adds no requests beyond the consolidated review. Thanks for working through this.