fix(agent): use instruction credentials, flag demonstrated unauth access, reconcile narrative (#322)

Three related gaps let authorized findings slip through a headless scan:

1. Credentials/tech hints passed via --instruction were treated as
   background, so the agent gave up at a 401 instead of driving the
   login flow. Add an operational principle that provided credentials
   and technology hints MUST be used to authenticate before concluding
   anything about the authenticated surface, with the login
   request/response as evidence either way.

2. Demonstrated unauthenticated access (e.g. an open Docker Registry
   serving GET /v2/_catalog and image pull/push) was dismissed as
   "configuration hygiene". Carve out in both the system prompt and the
   severity-calibration skill: the bare absence of an auth header is an
   observation, but proven anonymous access to a protected operation or
   data is a reportable finding scored on the access it grants, with
   missing auth as an aggravating factor.

3. A report narrative could describe weaknesses while the vulnerability
   count stayed zero. Add _narrative_reconciliation to finish_scan: a
   zero-finding report whose prose contains weakness language rides back
   a narrative_warning so the contradiction is visible in the run
   record, plus a reconciliation gate in the finish_scan docstring.

Tests cover the narrative-reconciliation helper.
This commit is contained in:
ljluestc 2026-10-27 00:09:50 -07:00
parent ef5d810047
commit bb0aa9a606
4 changed files with 140 additions and 2 deletions

View file

@ -152,6 +152,7 @@ OPERATIONAL PRINCIPLES:
- For skills not preloaded, use `load_skill` to pull them inline — prefer loading the matching skill before guessing payloads, workflows, or tool syntax from memory
- Use custom Python or shell code when you want to dig deeper, automate custom workflows, batch operations, triage results, build target-specific validation, or do work that existing tools do not cover cleanly
- Chain related weaknesses when needed to demonstrate real impact
- ACT ON USER-PROVIDED CREDENTIALS AND HINTS: when the user instructions name credentials (even "default" or "may be" ones like `admin:prom-operator`), a technology ("this is a Grafana dashboard", "Docker Registry v2 API"), or an auth scheme, treat them as working intelligence to USE, not just background. Authenticate with them before concluding anything about the authenticated surface: identify the login mechanism for that technology (e.g. `POST /api/login` for Grafana, `POST /login`, HTTP Basic, a bearer-token exchange, or the browser login form), send the real login request, confirm you hold a valid session/cookie/token, and only then test post-auth surfaces. A bare `401`/`403` on a protected endpoint is NOT a dead end when credentials were provided — it means you have not authenticated yet. Do not report "credential issues" or "all attempts returned 401" until you have actually driven the login flow with the given credentials and shown it failed; if the credentials genuinely work, proceed with authenticated testing, and if they genuinely do not, say so with the login request/response as evidence. Record a confirmed working credential set in `notes` so other agents reuse it.
- Consider business logic and context in validation
- Use think for non-trivial planning, uncertainty, multi-step security work, or choosing what to do next. Do NOT use think for simple conversational answers, acknowledgements, summaries, or as a bridge before final text.
- WORK METHODICALLY - Don't stop at shallow checks when deeper in-scope validation is warranted
@ -201,7 +202,8 @@ VALIDATION REQUIREMENTS:
- Full validation required - no assumptions
- Demonstrate concrete impact with evidence
- Consider business context for severity assessment — check whether the target is a demo/sandbox environment or content meant to be public, and factor that in
- Score only the security impact demonstrated by the proof of concept. Reachability, missing authentication, scanner labels, and theoretical follow-on attacks do not by themselves justify non-None CVSS impact metrics
- Score only the security impact demonstrated by the proof of concept. Reachability, the bare absence of an auth header, scanner labels, and theoretical follow-on attacks do not by themselves justify non-None CVSS impact metrics
- BUT missing authentication is a real, reportable finding the moment you DEMONSTRATE it grants access to an operation or data that is meant to be protected — not a mere observation. If a service that should require auth lets you perform a sensitive action unauthenticated and you prove it with a request/response, file it and score the impact you demonstrated. Examples: an open Docker Registry where `GET /v2/_catalog` enumerates images and an unauthenticated pull (or, worse, push/overwrite) succeeds; an admin/API/dashboard endpoint returning protected data to an anonymous caller; a database, management port, or metrics/debug endpoint exposed with no auth. "No auth configured" that you confirm against a surface that should have it is unauthorized access (read) or unauthorized modification (write/push) — report it, don't dismiss it as configuration hygiene. The credential-less position here is what makes it worse, not what makes it a non-finding.
- Treat public metadata, internal-looking identifiers, source maps without secrets, and transport/configuration hygiene as observations unless validation proves unauthorized restricted-data access, modification, or service disruption
- Every non-None Confidentiality, Integrity, or Availability metric must map to explicit evidence in the report; use Scope Changed only for a demonstrated crossing of security authorities
- Independent verification through subagent

View file

@ -92,6 +92,30 @@ circumstances to exceed medium:
how to obtain it, this is usually low/medium.
- Enumeration that only confirms an account, domain, or version exists.
## Missing Authentication: Observation vs. Demonstrated Access
Do not confuse "an endpoint has no auth header" with "unauthenticated
access to something protected". The first, on its own, is an
observation. The second is a real finding, scored on what the access
lets an attacker do:
- An unauthenticated service that is *supposed* to be protected and that
you demonstrate serving sensitive data, admin actions, or write
operations to an anonymous caller is unauthorized access — rate it on
the demonstrated read/write impact, not down to informational. An open
Docker Registry allowing catalog enumeration and image pull is at
least a real confidentiality finding; one allowing unauthenticated
push/overwrite (supply-chain tampering) rises toward high/critical.
- The absence of required authentication is an *aggravating* factor
(`PR:N` on a surface that should require privileges), not a reason to
withhold the finding. A bare `alg:none` token, an anonymous admin API,
or an exposed management/metrics/debug port each earns severity from
the access it grants, demonstrated with a request/response pair.
What stays informational is missing auth with nothing sensitive behind
it, or an endpoint that is *intended* to be public. Demonstrate what the
anonymous request actually returns or changes, then rate that.
## Downgrade, Don't Delete
A finding that turns out to be constrained gets a lower severity — not a

View file

@ -65,6 +65,11 @@ def _do_finish(
)
vuln_count = len(report_state.vulnerability_reports)
coverage_summary = _coverage_summary(agent_graph)
narrative_summary = _narrative_reconciliation(
vuln_count=vuln_count,
executive_summary=executive_summary,
technical_analysis=technical_analysis,
)
except (ImportError, AttributeError) as e:
logger.exception("finish_scan persistence failed")
return {"success": False, "error": f"Failed to complete scan: {e!s}"}
@ -80,9 +85,68 @@ def _do_finish(
"vulnerabilities_found": vuln_count,
}
result.update(coverage_summary)
result.update(narrative_summary)
return result
# Weakness-describing language that, in a report narrative paired with a zero
# vulnerability count, signals findings that were written up in prose but never
# filed as formal records (every one of them uncredited and untracked).
_WEAKNESS_MARKERS: tuple[str, ...] = (
"missing security header",
"unsecured",
"unauthenticated",
"no authentication",
"without authentication",
"exposed",
"open registry",
"default credential",
"misconfigur",
"vulnerab",
"injection",
"disclosure",
"weak ",
"insecure",
"could allow",
"allows an attacker",
)
def _narrative_reconciliation(
*,
vuln_count: int,
executive_summary: str,
technical_analysis: str,
) -> dict[str, Any]:
"""Flag a zero-count report whose prose nonetheless describes weaknesses.
A scan whose narrative lists missing headers, an unsecured registry, or
other issues while the vulnerability tracker shows zero is contradicting
itself: those issues were described but never filed via the report tool, so
they are neither tracked, credited, nor reflected in the exit code. The
runtime cannot file them retroactively, but it can refuse to let the
contradiction ship silently — the warning rides back on the finish_scan
response so the mismatch is visible in the run record.
"""
if vuln_count > 0:
return {}
haystack = f"{executive_summary}\n{technical_analysis}".lower()
hits = sorted({marker.strip() for marker in _WEAKNESS_MARKERS if marker in haystack})
if not hits:
return {}
return {
"narrative_warning": (
"The report narrative describes weaknesses but zero vulnerabilities were "
"filed. Any real issue mentioned in the narrative must be filed as a formal "
"record via the report tool — otherwise it is untracked, uncredited, and "
"contradicts the finding count and exit code. File the missing findings (or, "
"if the narrative overstates and nothing is actually a vulnerability, reword "
"it so it no longer reads as one) before the report goes out."
),
"narrative_weakness_markers": hits,
}
def _coverage_summary(agent_graph: dict[str, Any]) -> dict[str, Any]:
"""Coverage counts, unresolved surfaces, and gaps the runtime can see.
@ -206,6 +270,20 @@ async def finish_scan(
record it as ``needs_follow_up`` before finishing. The response
from this tool reports coverage counts and any unresolved rows.
6. **Narrative reconciliation.** Before finishing, reconcile the prose
you are about to submit against the findings actually on file via
``list_reports``. Every weakness your ``executive_summary`` or
``technical_analysis`` describes — a missing security header, an
unauthenticated/open service, an exposed endpoint — must correspond
to a filed report; describing an issue in the narrative while the
vulnerability count stays zero is a contradiction (the issue is then
untracked, uncredited, and the ``0``/clean exit code misreports the
run). If the narrative names an issue that is real, file it with the
report tool first; if it overstates and nothing is actually a
vulnerability, reword the narrative so it no longer reads as one. The
response flags this mismatch when it detects weakness language with
zero findings filed.
**Calling this multiple times overwrites the previous report.**
Make the single call comprehensive.

View file

@ -7,7 +7,7 @@ from typing import TYPE_CHECKING
import pytest
from strix.tools.coverage.tools import _record_impl, hydrate_coverage_from_disk
from strix.tools.finish.tool import _coverage_summary
from strix.tools.finish.tool import _coverage_summary, _narrative_reconciliation
if TYPE_CHECKING:
@ -63,3 +63,37 @@ def test_an_empty_ledger_still_warns_first() -> None:
assert summary["coverage_recorded"] == 0
assert "No coverage was recorded" in summary["coverage_warning"]
def test_narrative_describing_weaknesses_with_zero_findings_is_flagged() -> None:
summary = _narrative_reconciliation(
vuln_count=0,
executive_summary="The host exposes an unsecured Docker Registry.",
technical_analysis="Missing security headers were observed on all responses.",
)
assert "narrative_warning" in summary
assert set(summary["narrative_weakness_markers"]) >= {
"unsecured",
"missing security header",
}
def test_zero_findings_with_clean_narrative_raises_no_warning() -> None:
summary = _narrative_reconciliation(
vuln_count=0,
executive_summary="The assessed surfaces were reviewed and no issues were found.",
technical_analysis="All tested endpoints enforced authentication correctly.",
)
assert summary == {}
def test_filed_findings_suppress_narrative_warning() -> None:
summary = _narrative_reconciliation(
vuln_count=3,
executive_summary="An unauthenticated registry was exposed.",
technical_analysis="Multiple vulnerabilities confirmed.",
)
assert summary == {}