mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(config): report every permissive setting in a file, not the first one
NodeGoat's server.js configures a permissive session at line 78 and disables template
auto-escaping at line 135, both inside the same function. The arbiter returned one verdict per
question, so the escaping defect that task 14.3 added was detected on every run and reported on
none: which of two real defects a contributor saw depended on where in the file it sat.
Configuration now returns one verdict per setting whose value the model could read. The pipeline
already emitted one finding per entailed answer, so this arbiter was the only place still folding
them to one.
Three boundaries are drawn rather than assumed.
- A setting whose value is COMPUTED stays one verdict per region. That band is a question for the
judge rather than a finding, and returning every undecided setting would multiply model calls
for claims the graph did not establish.
- A call carrying two permissive literals is one finding. cors({origin: true, credentials: true})
is one misconfiguration, not two.
- Config verdicts now carry a path:line:function location. Without one they shared the region's
identity and the deduplicator kept a single finding, so the plural answer collapsed straight
back to one. That is the same gap dominance carried until its findings stopped arriving with no
line at all.
Dominance is left answering once per region, and that is settled rather than pending: its claim is
that a region contradicts itself, so the region is the unit and a second verdict would restate the
same inconsistency. The pipeline now says so where it used to call this a known gap.
Measured: NodeGoat's config_secrets goes from one finding to two, server.js:78 for the session and
server.js:135 for the disabled escaping, with the applied marked.setOptions fix between them
staying silent. VAmPI is unchanged and no other family moved.
1,417 tests pass, seventeen skipped; thirteen Joern-gated query tests pass in the image.
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01EmM1j46qgKMh3zqD1QMwEo
This commit is contained in:
Marius Ciepluch
co-authored by
Claude Opus 5
Claude Opus 5
parent
e6bdac46e2
commit
3bd844055c
@@ -183,6 +183,11 @@ public routes and both wrong, and an application-wide scope entails four true de
|
||||
same two wrong ones. Neither scope can tell a public route from a protected one, so the carried
|
||||
rung stays capped and that question stays with the judge.
|
||||
|
||||
Configuration reports every setting it decides, not the first one in the file. NodeGoat's
|
||||
server.js now carries both its permissive session and its disabled template auto-escaping, each
|
||||
with its own site; the escaping defect had been detected on every run since task 14.3 and
|
||||
reported on none, because an earlier setting in the same function won the single verdict.
|
||||
|
||||
Closing the second gap surfaced four defects, three of which were false positives it would
|
||||
otherwise have shipped: a permissive-value set shared across settings whose booleans have
|
||||
opposite polarity, a dotted setting matched on its trailing segment alone, a flow query asked of
|
||||
@@ -196,4 +201,5 @@ Evidence: [family detection census](../../../benchmarks/measurements/2026-09-19-
|
||||
[output encoding family](../../../benchmarks/measurements/2026-09-19-output-encoding-family.json),
|
||||
[access control across the module split](../../../benchmarks/measurements/2026-09-20-access-control-module-split.json),
|
||||
[collection read obligation](../../../benchmarks/measurements/2026-09-20-collection-read-obligation.json),
|
||||
[sibling scope and identity](../../../benchmarks/measurements/2026-09-20-sibling-scope-and-identity.json).
|
||||
[sibling scope and identity](../../../benchmarks/measurements/2026-09-20-sibling-scope-and-identity.json),
|
||||
[every permissive setting](../../../benchmarks/measurements/2026-09-20-every-permissive-setting.json).
|
||||
|
||||
@@ -491,10 +491,13 @@ Preserve all completed task evidence below.
|
||||
- _Boundary: shipped semantic facts, family taxonomy_
|
||||
- _Depends: 14.1_
|
||||
- _Requirements: 6.2, 8.1_
|
||||
- [ ] 14.6 Report every permissive setting in a file, not the first one
|
||||
- [x] 14.6 Report every permissive setting in a file, not the first one
|
||||
- **Named by the 14.3 measurement.** The configuration arbiter returns one verdict per question. NodeGoat's `server.js` holds a permissive session at line 78 and template auto-escaping disabled at line 135 in the same function, so the escaping defect is detected — measured model_entailed against the real query and the real arbiter — and never reported. A contributor is shown one of two defects and told nothing about the other.
|
||||
- This is the arbiter's contract, not a data gap: `verdict()` returns `Verdict | None` and the scan collects one per question. Changing it touches every family that returns a verdict, so it is scheduled rather than folded into a vocabulary change.
|
||||
- Done when a file with two permissive settings reports both, with their own sites and witnesses, and no family reports a defect twice.
|
||||
- _Verified 2026-09-20:_ 1,417 tests passed, seventeen skipped; thirteen Joern-gated query tests pass in the image; Ruff/format/mypy passed. Configuration now returns one verdict per setting whose value the model could read. The pipeline already emitted one finding per entailed answer, so only this arbiter was still folding them to one. NodeGoat's `server.js` reports both defects with their own sites, `server.js:78` for the permissive session and `server.js:135` for the disabled template auto-escaping, which is the defect task 14.3 could detect and never report. Its applied fix at line 126 stays silent. VAmPI is unchanged and no other family moved. Evidence: `benchmarks/measurements/2026-09-20-every-permissive-setting.json`.
|
||||
- _Three boundaries drawn rather than assumed._ A setting whose value is COMPUTED stays one verdict per region: that band is a question for the judge, and returning every undecided setting would multiply model calls for claims the graph did not establish. A call carrying two permissive literals is one finding, because `cors({origin: true, credentials: true})` is one misconfiguration. And config verdicts now carry a `path:line:function` location, without which the plural answer collapsed straight back to one in the deduplicator, which is the same gap dominance carried until its findings stopped arriving with no line.
|
||||
- _Dominance is left answering once per region, and that is settled rather than pending._ Its claim is that a region contradicts itself, so the region is the unit and a second verdict would restate the same inconsistency. The pipeline says so where it used to call this a known gap.
|
||||
- _Boundary: contributor-scan arbitration, Rung ladder_
|
||||
- _Depends: 14.3_
|
||||
- _Requirements: 6.3, 8.1_
|
||||
|
||||
@@ -0,0 +1,51 @@
|
||||
{
|
||||
"schema_version": 1,
|
||||
"experiment": "every-permissive-setting-v1",
|
||||
"date": "2026-09-20",
|
||||
"task": "pre-push-safety-net 14.6",
|
||||
"claim_scope": "One change to the configuration arbiter's contract, measured on two subjects. Detection only. No capability qualified.",
|
||||
"problem": "The arbiter returned one verdict per question. NodeGoat's server.js configures a permissive session at line 78 and disables template auto-escaping at line 135, both inside the same function, so the escaping defect that task 14.3 added was detected on every run and reported on none. Which of two real defects a contributor saw depended on where in the file it sat.",
|
||||
"change": {
|
||||
"plural_where_the_graph_decided": "config_value.verdicts returns one verdict per setting whose value the model could read. The pipeline already emitted one finding per entailed answer; only configuration was still folding them to one.",
|
||||
"singular_where_it_did_not": "A setting whose value is COMPUTED stays one verdict per region. That band is a question for the judge rather than a finding, and returning every undecided setting would multiply model calls for claims the graph did not establish.",
|
||||
"one_defect_per_setting": "A call with two permissive literals is one finding, not two. `cors({origin: true, credentials: true})` is one misconfiguration.",
|
||||
"sites": "Config verdicts now carry a location, path:line:function. Without one they shared the region's identity and the deduplicator kept a single finding, so the plural answer would have collapsed back to one. The same gap dominance carried until its findings stopped arriving with no line.",
|
||||
"dominance_left_alone": "Its claim is that a region CONTRADICTS ITSELF, so the region is the unit and a second verdict would restate the same inconsistency. Recorded in the pipeline as settled rather than pending."
|
||||
},
|
||||
"measured": {
|
||||
"nodegoat": {
|
||||
"config_secrets_before": 1,
|
||||
"config_secrets_after": 2,
|
||||
"now_reported": [
|
||||
{
|
||||
"site": "server.js:78:<lambda>0",
|
||||
"witness": "session({...}): permissive literal true"
|
||||
},
|
||||
{
|
||||
"site": "server.js:135:<lambda>0",
|
||||
"witness": "swig.setDefaults({ autoescape: false }): permissive literal false",
|
||||
"note": "the template auto-escaping defect task 14.3 could detect and never report"
|
||||
}
|
||||
],
|
||||
"still_silent": "marked.setOptions({ sanitize: true }) at line 126, the applied fix between the two",
|
||||
"other_families": {
|
||||
"access_control": 0,
|
||||
"injection": 3,
|
||||
"output_encoding": 1,
|
||||
"path": 0,
|
||||
"prototype": 0,
|
||||
"untrusted_destination": 2
|
||||
}
|
||||
},
|
||||
"vampi": {
|
||||
"config_secrets": 1,
|
||||
"unchanged": true
|
||||
},
|
||||
"sites_are_distinct": true
|
||||
},
|
||||
"limits": [
|
||||
"Only the decided band is plural; a file with two computed settings still asks one question.",
|
||||
"The location uses the region's file, which is correct because this arbiter never leaves the file it was asked about.",
|
||||
"Two subjects, both used to develop this analyser."
|
||||
]
|
||||
}
|
||||
@@ -30,17 +30,29 @@ def request_params(spec: ConfigSpec, *, function: str = "", file: str = "") -> d
|
||||
return {"settings": spec.settings, "function": function, "file": file}
|
||||
|
||||
|
||||
def verdict(cpg: CpgResult, spec: ConfigSpec, *, function: str = "", file: str = "") -> Verdict | None:
|
||||
"""The verdict for a security setting in ``function``, or ``None`` when nothing is established."""
|
||||
def verdicts(cpg: CpgResult, spec: ConfigSpec, *, function: str = "", file: str = "") -> list[Verdict]:
|
||||
"""Every configuration defect this region establishes, one per setting, strongest first.
|
||||
|
||||
A file configures many things, and one verdict per region reported the first of them. Measured on
|
||||
NodeGoat: `server.js` disables template auto-escaping at line 135 and configures a permissive session at
|
||||
line 78, both inside the same function, so the escaping defect was detected on every run and reported on
|
||||
none. A contributor was shown one of two defects and told nothing about the other, and which one depended
|
||||
on where in the file it sat.
|
||||
|
||||
Only the decided band is plural. A setting whose value the model could not READ is a question for the
|
||||
judge rather than a finding, and asking it once per region is what that band has always cost; returning
|
||||
every undecided setting would multiply model calls for claims none of which the graph established.
|
||||
"""
|
||||
rows = cpg.run("config", request_params(spec, function=function, file=file))
|
||||
settings = _settings(rows, spec, function=function)
|
||||
if not settings:
|
||||
return None
|
||||
return []
|
||||
|
||||
# Both abstractions read the same literal: a permissive flag ("*", True) and a weak algorithm ("md5").
|
||||
permissive = {value.strip().strip("'\"").lower() for value in spec.permissive}
|
||||
permissive |= {value.strip().strip("'\"").lower() for value in spec.weak_algorithms}
|
||||
scoped = {setting: {value.strip().strip("'\"").lower() for value in values} for setting, values in spec.permissive_by_setting}
|
||||
found: list[Verdict] = []
|
||||
for row in settings:
|
||||
literals = row["literals"]
|
||||
if not isinstance(literals, list):
|
||||
@@ -48,22 +60,51 @@ def verdict(cpg: CpgResult, spec: ConfigSpec, *, function: str = "", file: str =
|
||||
allowed = _permissive_for(str(row["setting"]), scoped) or permissive
|
||||
for literal in literals:
|
||||
if str(literal).strip().strip("'\"").lower() in allowed:
|
||||
return Verdict(
|
||||
rung=Rung.ENTAILED,
|
||||
family=spec.family,
|
||||
witness=f"{row['setting']} (line {row['line']}): permissive literal {literal}",
|
||||
found.append(
|
||||
Verdict(
|
||||
rung=Rung.ENTAILED,
|
||||
family=spec.family,
|
||||
witness=f"{row['setting']} (line {row['line']}): permissive literal {literal}",
|
||||
location=_location(row, file),
|
||||
)
|
||||
)
|
||||
break # one verdict per setting: a call with two permissive literals is one defect
|
||||
if found:
|
||||
return found
|
||||
|
||||
# A setting whose value the model could not read. Not safe, not decided.
|
||||
computed = [row for row in settings if not row["literals"]] # type: ignore[truthy-iterable]
|
||||
if computed:
|
||||
row = computed[0]
|
||||
return Verdict(
|
||||
rung=Rung.CORROBORATED,
|
||||
family=spec.family,
|
||||
witness=f"{row['setting']} (line {row['line']}): value is computed, not a literal the model can evaluate",
|
||||
)
|
||||
return None
|
||||
return [
|
||||
Verdict(
|
||||
rung=Rung.CORROBORATED,
|
||||
family=spec.family,
|
||||
witness=f"{row['setting']} (line {row['line']}): value is computed, not a literal the model can evaluate",
|
||||
location=_location(row, file),
|
||||
)
|
||||
]
|
||||
return []
|
||||
|
||||
|
||||
def verdict(cpg: CpgResult, spec: ConfigSpec, *, function: str = "", file: str = "") -> Verdict | None:
|
||||
"""The strongest single verdict, kept for callers that ask one question and want one answer."""
|
||||
answers = verdicts(cpg, spec, function=function, file=file)
|
||||
return answers[0] if answers else None
|
||||
|
||||
|
||||
def _location(row: Mapping[str, object], file: str) -> str:
|
||||
"""``path:line:function`` of the setting, when the region named a file.
|
||||
|
||||
Two settings in one file are two findings only if they are two SITES. Without this they shared the
|
||||
region's own identity and the deduplicator kept one, which is the same defect dominance carried until its
|
||||
findings stopped arriving as `api_views/users.py:?:update_password`. The config query reports no file of
|
||||
its own, so the region's is used -- correct because this arbiter never leaves the file it was asked about.
|
||||
"""
|
||||
line = str(row.get("line") or "")
|
||||
if not file or not line.lstrip("-").isdigit() or int(line) < 0:
|
||||
return ""
|
||||
return f"{file}:{line}:{row.get('method') or '?'}"
|
||||
|
||||
|
||||
def _permissive_for(setting: str, scoped: Mapping[str, set[str]]) -> set[str]:
|
||||
@@ -115,4 +156,4 @@ def _settings(rows: object, spec: ConfigSpec, *, function: str) -> list[dict[str
|
||||
return sorted(kept, key=lambda row: (str(row["method"]), _line(row), str(row["setting"])))
|
||||
|
||||
|
||||
__all__ = ["request_params", "verdict"]
|
||||
__all__ = ["request_params", "verdict", "verdicts"]
|
||||
|
||||
@@ -30,6 +30,7 @@ from typing import Any
|
||||
from ..cpg.backend import CpgResult
|
||||
from ..redaction import redact_secrets
|
||||
from .config_value import verdict as config_verdict
|
||||
from .config_value import verdicts as config_verdicts
|
||||
from .dominance import verdict as dominance_verdict
|
||||
from .ladder import Rung, Verdict
|
||||
from .specs import ConfigSpec, DominanceSpec, TaintSpec
|
||||
@@ -83,11 +84,15 @@ def _arbitrate_all(
|
||||
One verdict per region was the right answer for a pair -- one labelled function, one bug -- and the wrong
|
||||
one for a region that spans a file. A PHP file with an SQL injection on line 6 and a command injection on
|
||||
line 13 produced ONE injection verdict, and `system` won it on flow length, so the SQL injection went
|
||||
unreported by construction. Only taint can report several sites today; dominance and config still answer
|
||||
once per region, which is correct for dominance (its claim is about the region's consistency) and a
|
||||
known gap for config.
|
||||
unreported by construction.
|
||||
|
||||
Taint and configuration report every site they decide. Dominance still answers once per region, and that
|
||||
is correct rather than pending: its claim is that a region CONTRADICTS ITSELF, so the region is the unit
|
||||
and a second verdict would be the same statement about the same inconsistency.
|
||||
"""
|
||||
if isinstance(spec, (DominanceSpec, ConfigSpec)):
|
||||
if isinstance(spec, ConfigSpec):
|
||||
return config_verdicts(cpg, spec, function=function, file=path)
|
||||
if isinstance(spec, DominanceSpec):
|
||||
answer = _arbitrate(cpg, spec, path=path, function=function, parameter_sources=parameter_sources, call_depth=call_depth)
|
||||
return [answer] if answer is not None else []
|
||||
return taint_verdicts(cpg, spec, function=function, file=path, parameter_sources=parameter_sources, call_depth=call_depth)
|
||||
|
||||
@@ -214,3 +214,44 @@ def test_the_first_setting_in_a_file_is_the_one_reported() -> None:
|
||||
]
|
||||
answer = verdict(_cpg(rows), _javascript_spec(), function="m")
|
||||
assert answer is not None and "line 78" in answer.witness, answer.witness
|
||||
|
||||
|
||||
def test_a_file_reports_every_permissive_setting_not_the_first() -> None:
|
||||
"""Two defects in one file are two findings, with their own sites.
|
||||
|
||||
Measured on NodeGoat: `server.js` configures a permissive session at line 78 and disables template
|
||||
auto-escaping at line 135, both inside the same function. One verdict per region reported the session and
|
||||
the escaping defect was detected on every run and reported on none, so which of two real defects a
|
||||
contributor saw depended on where in the file it sat.
|
||||
"""
|
||||
from openultrasast.model.config_value import verdicts
|
||||
from openultrasast.model.ladder import Rung
|
||||
|
||||
rows = [
|
||||
{"setting": "session({ resave: true })", "line": "78", "method": "<lambda>0", "literalArgs": ["true"], "args": []},
|
||||
{"setting": "marked.setOptions({ sanitize: true })", "line": "126", "method": "<lambda>0", "literalArgs": ["true"], "args": []},
|
||||
{"setting": "swig.setDefaults({ autoescape: false })", "line": "135", "method": "<lambda>0", "literalArgs": ["false"], "args": []},
|
||||
]
|
||||
answers = verdicts(_cpg(rows), _javascript_spec(), function="<lambda>0", file="server.js")
|
||||
assert len(answers) == 2, [a.witness for a in answers]
|
||||
assert all(a.rung is Rung.ENTAILED for a in answers)
|
||||
assert [a.location for a in answers] == ["server.js:78:<lambda>0", "server.js:135:<lambda>0"]
|
||||
# The applied fix in between stays silent, which is what the per-setting polarity is for.
|
||||
assert all("marked" not in a.witness for a in answers)
|
||||
|
||||
|
||||
def test_one_setting_with_two_permissive_literals_is_one_finding() -> None:
|
||||
"""A call is a defect once, however many of its values are open. No family reports a defect twice."""
|
||||
from openultrasast.model.config_value import verdicts
|
||||
|
||||
rows = [
|
||||
{
|
||||
"setting": "cors({ origin: true, credentials: true })",
|
||||
"line": "9",
|
||||
"method": "app",
|
||||
"literalArgs": ["true", "true"],
|
||||
"args": [],
|
||||
}
|
||||
]
|
||||
answers = verdicts(_cpg(rows), _javascript_spec(), function="app", file="server.js")
|
||||
assert len(answers) == 1, [a.witness for a in answers]
|
||||
|
||||
Reference in New Issue
Block a user