Skip to content

Commit 39b9023

Browse files
committed
Let the second look fetch what a claim depends on
Fourteen reasoning findings re-checked on production came back unconfirmed, nearly all for one reason: the check, if there is one, lives in code the second look was not shown. A router level dependency, a helper in another file, or a lookup that may scope by owner. The second look now fetches the bodies of the access checks and lookups the handler calls from elsewhere in the repository, preferring the module a call names, so rbac.in_scope comes from rbac.py and not from a namesake. It also gets the router, app and middleware lines that wrap the route, in its own file and where its router is mounted. Like the permission lines, none of this is part of the cache key, so unchanged code is not asked about again. Check the unconfirmed ones again on the AI page takes the findings set aside as unconfirmed through that look once. One it now confirms goes back in the work list with its write-up; a guarded one is set aside naming the check. The AI page's code reasoning section links to these buttons.
1 parent 3e888e4 commit 39b9023

3 files changed

Lines changed: 183 additions & 16 deletions

File tree

‎app/core/code_reasoning.py‎

Lines changed: 166 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@
8989
HELPER_CHARS = 2_500 # access check bodies sent beside one handler
9090
HELPERS_PER_UNIT = 3
9191
ANSWER_TOKENS = 900
92-
VERIFY_TOKENS = 300
92+
VERIFY_TOKENS = 350
9393
# The write-up a developer reads, asked once per kept finding and cached with
9494
# it, so a fork, a branch or the next scan reuses it for nothing.
9595
WRITEUP_TOKENS = 1_100
@@ -642,6 +642,135 @@ def permission_context(unit: Unit, root: Path) -> str:
642642
return "\n".join(hits)
643643

644644

645+
# What the second look may fetch beyond the handler: the bodies of functions
646+
# it calls that were not sent, and what wraps the route. Both stay out of the
647+
# cache key, so unchanged code is never asked about again because of them.
648+
DEFINITIONS = 5
649+
DEFINITION_LINES = 24
650+
DEFINITION_CHARS = 6_000
651+
WRAPPER_CHARS = 2_000
652+
_LOOKUPISH = re.compile(r"(?i)(get|find|fetch|load|lookup|retrieve|read|by_id|query|owned|"
653+
r"scoped|for_user|visible)")
654+
_COMMON = {"len", "str", "int", "float", "dict", "list", "set", "tuple", "print", "isinstance",
655+
"range", "get", "add", "append", "update", "filter", "first", "all", "json", "items",
656+
"keys", "values", "format", "join", "split", "strip", "lower", "upper", "commit",
657+
"flush", "execute", "one", "one_or_none", "scalar", "sorted", "min", "max", "sum",
658+
"any", "open", "super", "type", "bool", "enumerate", "zip", "map", "next", "iter",
659+
"hasattr", "getattr", "setattr", "push", "then", "catch", "send",
660+
"status", "redirect", "render", "jsonify", "Response", "JSONResponse"}
661+
_WRAPS = re.compile(r"(APIRouter\(|Blueprint\(|include_router\(|register_blueprint|"
662+
r"before_request|add_middleware|\.use\(|dependencies\s*=|Route::|"
663+
r"->middleware\(|@login_required|@requires_auth|@permission_required|"
664+
r"permission_classes|authentication_classes|UseGuards|\[Authorize)")
665+
666+
667+
def _def_pattern(name: str, lang: str):
668+
n = re.escape(name)
669+
if lang == "python":
670+
return re.compile(rf"^\s*(async\s+)?def\s+{n}\s*\(")
671+
if lang == "php":
672+
return re.compile(rf"function\s+&?\s*{n}\s*\(")
673+
return re.compile(rf"(function\s+{n}\s*\(|(const|let|var)\s+{n}\s*=|"
674+
rf"^\s*(async\s+)?{n}\s*\([^)]*\)\s*\{{|{n}\s*:\s*(async\s*)?function)")
675+
676+
677+
def _source_files(root: Path, lang: str):
678+
"""(relative path, lines) for each file of this language in the checkout."""
679+
seen = 0
680+
for path in sorted(Path(root).rglob("*")):
681+
if seen >= MAX_FILES:
682+
return
683+
if LANG_OF.get(path.suffix.lower()) != lang or path.is_symlink() or not path.is_file():
684+
continue
685+
rel = str(path.relative_to(root)).replace("\\", "/")
686+
if any(part in SKIP_DIRS or part.startswith(".") for part in Path(rel).parts[:-1]):
687+
continue
688+
try:
689+
if path.stat().st_size > MAX_FILE_BYTES:
690+
continue
691+
text = path.read_text(encoding="utf-8", errors="replace")
692+
except OSError:
693+
continue
694+
seen += 1
695+
yield rel, text.split("\n")
696+
697+
698+
def definitions_context(unit: Unit, root: Path) -> str:
699+
"""The bodies of functions the handler calls that were not sent with it.
700+
701+
Access checks first, then the lookups that might scope by owner, so a
702+
claim that "nothing checks the owner" can be tested against get_doc or
703+
find_for_user rather than guessed about.
704+
"""
705+
have = {n for n, _ in unit.helpers} | {n for n, _ in unit.extra} | {unit.name}
706+
names = []
707+
for m in _CALLS.finditer(unit.text):
708+
name = m.group(1)
709+
if name in have or name in _COMMON or len(name) < 3 or name in names:
710+
continue
711+
if name == "require" and Path(unit.path).suffix.lower() in (".js", ".mjs", ".cjs", ".ts"):
712+
continue # loads a module there, it checks nothing
713+
names.append(name)
714+
names.sort(key=lambda n: (0 if _AUTHISH.search(n) else 1 if _LOOKUPISH.search(n) else 2))
715+
names = [n for n in names if _AUTHISH.search(n) or _LOOKUPISH.search(n)][:DEFINITIONS]
716+
if not names:
717+
return ""
718+
lang = LANG_OF.get(Path(unit.path).suffix.lower())
719+
patterns = {n: _def_pattern(n, lang) for n in names}
720+
# rbac.in_scope is the one in rbac.py, not a namesake somewhere else.
721+
qualified = {m.group(2): m.group(1).split(".")[-1]
722+
for m in re.finditer(r"\b([\w.]+)\.(\w+)\s*\(", unit.text)}
723+
wanted_stems = {qualified[n] for n in names if n in qualified}
724+
files = sorted(_source_files(root, lang),
725+
key=lambda f: 0 if Path(f[0]).stem in wanted_stems else 1)
726+
found, total = {}, 0
727+
for rel, lines in files:
728+
for name, pattern in patterns.items():
729+
if name in found:
730+
continue
731+
if name in qualified and Path(rel).stem != qualified[name] and any(
732+
Path(f[0]).stem == qualified[name] for f in files):
733+
continue # the qualified home exists; do not settle for a namesake
734+
for i, line in enumerate(lines):
735+
if pattern.search(line):
736+
body = "\n".join(f"{j + 1}: {lines[j]}"
737+
for j in range(i, min(len(lines), i + DEFINITION_LINES)))
738+
chunk = f"# {name}, {rel}\n{body}"
739+
if total + len(chunk) <= DEFINITION_CHARS:
740+
found[name] = chunk
741+
total += len(chunk)
742+
break
743+
if len(found) == len(names):
744+
break
745+
return "\n".join(found[n] for n in names if n in found)
746+
747+
748+
def wrappers_context(unit: Unit, root: Path) -> str:
749+
"""What wraps the route: router and app guards, in its file and where it is mounted."""
750+
lang = LANG_OF.get(Path(unit.path).suffix.lower())
751+
stem = Path(unit.path).stem
752+
own, mounts, total = [], [], 0
753+
for rel, lines in _source_files(root, lang):
754+
for i, line in enumerate(lines, 1):
755+
if not _WRAPS.search(line):
756+
continue
757+
if rel == unit.path:
758+
if unit.start <= i <= unit.end:
759+
continue
760+
chunk = f"{rel}:{i}: {line.strip()[:200]}"
761+
target = own
762+
elif re.search(rf"\b{re.escape(stem)}\b", line):
763+
chunk = f"{rel}:{i}: {line.strip()[:200]}"
764+
target = mounts
765+
else:
766+
continue
767+
if total + len(chunk) > WRAPPER_CHARS:
768+
break
769+
target.append(chunk)
770+
total += len(chunk)
771+
return "\n".join(own + mounts)
772+
773+
645774
# ---- asking -------------------------------------------------------------------
646775

647776
def digest_of(unit: Unit) -> str:
@@ -794,7 +923,16 @@ def confirm(self, unit: Unit, item: dict, root: Path | None = None) -> dict:
794923
if context:
795924
prompt += _block(token, "where the permissions and roles it checks are defined",
796925
mask(context))
797-
hidden = unresolved_checks(unit)
926+
defined = definitions_context(unit, root) if root is not None else ""
927+
if defined:
928+
prompt += _block(token, "bodies of functions it calls, from elsewhere in the "
929+
"repository", mask(defined))
930+
wrapping = wrappers_context(unit, root) if root is not None else ""
931+
if wrapping:
932+
prompt += _block(token, "router, app and middleware lines that wrap this route",
933+
mask(wrapping))
934+
shown = defined + "\n" + "\n".join(b for _n, b in unit.helpers)
935+
hidden = [n for n in unresolved_checks(unit) if f"# {n}," not in shown]
798936
if hidden:
799937
prompt += ("Access checks it calls that you were not shown: "
800938
+ ", ".join(hidden) + "\n")
@@ -1293,7 +1431,10 @@ def _short_claim(finding) -> str:
12931431
return text.split(". ", 1)[1].strip() if ". " in text else text.strip()
12941432

12951433

1296-
def recheck_open(actor: str = "code-reasoning") -> dict:
1434+
RECHECKED_DEEPER = "Checked with definitions"
1435+
1436+
1437+
def recheck_open(actor: str = "code-reasoning", unconfirmed: bool = False) -> dict:
12971438
"""Give every open reasoning finding the second look it never had.
12981439
12991440
Findings filed before every claim was checked went on the model's first
@@ -1312,12 +1453,18 @@ def recheck_open(actor: str = "code-reasoning") -> dict:
13121453
if not is_on(cfg) or not cfg.get("ai.send_code"):
13131454
return dict(counts, stopped="code reasoning or sending code is switched off")
13141455
left = budget(db, cfg)["left"]
1315-
rows = (db.query(Finding.id, Finding.repository, Finding.branch)
1316-
.filter(Finding.engines == ENGINE, Finding.state == FindingState.OPEN,
1317-
Finding.excepted.is_(False))
1318-
.filter(Finding.triage_reason.is_(None)
1319-
| ~Finding.triage_reason.contains(RECHECKED))
1320-
.order_by(Finding.repository, Finding.branch, Finding.id).all())
1456+
query = (db.query(Finding.id, Finding.repository, Finding.branch)
1457+
.filter(Finding.engines == ENGINE, Finding.state == FindingState.OPEN,
1458+
Finding.excepted.is_(False)))
1459+
if unconfirmed:
1460+
# The ones the first re-check could not settle, once more, now
1461+
# that the second look can fetch what it was missing.
1462+
query = query.filter(Finding.triage == "unconfirmed",
1463+
~Finding.triage_reason.contains(RECHECKED_DEEPER))
1464+
else:
1465+
query = query.filter(Finding.triage_reason.is_(None)
1466+
| ~Finding.triage_reason.contains(RECHECKED))
1467+
rows = query.order_by(Finding.repository, Finding.branch, Finding.id).all()
13211468
groups = {}
13221469
for fid, repository, branch in rows:
13231470
groups.setdefault((repository, branch or ""), []).append(fid)
@@ -1358,6 +1505,7 @@ def recheck_open(actor: str = "code-reasoning") -> dict:
13581505
continue
13591506
written = run.writeup(unit, item) if verdict["v"] == "real" else None
13601507
today = utcnow().strftime("%Y-%m-%d")
1508+
marker = RECHECKED_DEEPER if unconfirmed else RECHECKED
13611509
with session_scope() as db:
13621510
f = db.get(Finding, fid)
13631511
if verdict["v"] == "real":
@@ -1366,17 +1514,23 @@ def recheck_open(actor: str = "code-reasoning") -> dict:
13661514
plain = CATEGORIES.get(category, ("", ""))[1]
13671515
f.description = (f"{plain}. {item['w']}\n\n"
13681516
f"{writeup_text(item)}")[:6000]
1369-
f.triage_reason = (f"{f.triage_reason} {RECHECKED} on {today}: "
1517+
if unconfirmed:
1518+
# Back in the work list, now it holds up.
1519+
f.triage = ""
1520+
f.triage_reason = (f"{f.triage_reason} {marker} on {today}: "
13701521
"a second look that tried to prove it wrong "
13711522
"could not.")[:2000]
13721523
counts["confirmed"] += 1
13731524
else:
13741525
guarded = verdict["v"] == "guarded"
13751526
f.triage = "likely_fp" if guarded else "unconfirmed"
13761527
f.triage_reason = (
1377-
(f"{RECHECKED} on {today}: a second look found what looks like "
1528+
(f"{marker} on {today}: a second look found what looks like "
13781529
f"the check, {verdict['guard']}. " if guarded else
1379-
f"{RECHECKED} on {today}: a second look could not confirm it "
1530+
f"{marker} on {today}: a second look could not confirm it "
1531+
"from the code it was shown, including the functions it calls "
1532+
"and what wraps the route. " if unconfirmed else
1533+
f"{marker} on {today}: a second look could not confirm it "
13801534
"from the code it was shown. ")
13811535
+ (verdict["why"] + " " if verdict["why"] else "")
13821536
+ "Set aside, not closed: a person should confirm and mark it "

‎app/web/routes_api.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5497,7 +5497,9 @@ async def recheck_reasoning(request: Request, user: User = Depends(current_user)
54975497
with session_scope() as db:
54985498
record(db, "ai.code_reasoning_recheck_requested", actor=user.username,
54995499
target="open reasoning findings", source_ip=client_ip(request))
5500-
threading.Thread(target=code_reasoning.recheck_open, args=(user.username,),
5500+
form = await request.form()
5501+
unconfirmed = str(form.get("unconfirmed", "")) == "1"
5502+
threading.Thread(target=code_reasoning.recheck_open, args=(user.username, unconfirmed),
55015503
name="reasoning-recheck", daemon=True).start()
55025504
return redirect("/ai?msg=" + quote(
55035505
"Checking the open reasoning findings again. The audit log says what "

‎app/web/templates/ai.html‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,9 @@
275275
last read: a branch still on the same commit is not even fetched. A
276276
connection can switch it off for its repositories. What it finds is
277277
filed for a person to confirm, and it never lowers or closes anything.
278-
It needs sending code switched on above.</p>
278+
It needs sending code switched on above.
279+
{% if can_write and values['ai.code_reasoning'] %}<a href="#recheck">Check the findings
280+
it already filed again</a>, below the Save button.{% endif %}</p>
279281
<label>Monthly token limit
280282
<input type="number" name="ai.code_reasoning_monthly_tokens" min="0"
281283
max="2000000000" value="{{ values['ai.code_reasoning_monthly_tokens'] }}">
@@ -308,7 +310,7 @@
308310
</form>
309311

310312
{% if can_write and values['ai.code_reasoning'] %}
311-
<div class="card">
313+
<div class="card" id="recheck">
312314
<div class="card-head">Check the reasoning findings again</div>
313315
<div class="card-body">
314316
<p class="faint small">Every reasoning claim now has to survive a second look
@@ -318,10 +320,19 @@
318320
up a developer needs, and the rest are set aside with the check that was
319321
found, never closed. About 5,000 tokens a finding, from the monthly
320322
limit, and a finding already checked again is skipped.</p>
321-
<form method="post" action="{{ base }}/ai/reasoning/recheck">
323+
<form method="post" action="{{ base }}/ai/reasoning/recheck" style="display:inline">
322324
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
323325
<button class="btn" type="submit">Check them again</button>
324326
</form>
327+
<form method="post" action="{{ base }}/ai/reasoning/recheck" style="display:inline">
328+
<input type="hidden" name="csrf_token" value="{{ csrf_token }}">
329+
<input type="hidden" name="unconfirmed" value="1">
330+
<button class="btn" type="submit">Check the unconfirmed ones again</button>
331+
</form>
332+
<p class="faint small">The second button takes the ones set aside as unconfirmed,
333+
and lets the second look fetch what they were missing: the bodies of the
334+
functions the handler calls, and the router and middleware that wrap it.
335+
Ones it now confirms go back in the work list with the write-up.</p>
325336
</div>
326337
</div>
327338
{% endif %}

0 commit comments

Comments
 (0)