Skip to content

Commit 44337b9

Browse files
Address CodeRabbit review and fix black formatting
Harden PCI env parsing, tighten sync script safety checks, make bridge fallback tests deterministic, and format files flagged by CI black. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 68c7d35 commit 44337b9

6 files changed

Lines changed: 72 additions & 47 deletions

File tree

application/tests/opencre_gap_analysis_test.py

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,9 @@ def setUp(self) -> None:
2525
sqla.create_all()
2626
self.collection = db.Node_collection()
2727

28-
def test_backfill_populates_secure_headers_pair_from_auto_linked_nodes(self) -> None:
28+
def test_backfill_populates_secure_headers_pair_from_auto_linked_nodes(
29+
self,
30+
) -> None:
2931
cre = self.collection.add_cre(
3032
defs.CRE(
3133
id="636-347",
@@ -54,17 +56,13 @@ def test_backfill_populates_secure_headers_pair_from_auto_linked_nodes(self) ->
5456
payload = json.loads(self.collection.get_gap_analysis_result(cache_key))
5557
self.assertIn("636-347", payload["result"])
5658
path = next(iter(payload["result"]["636-347"]["paths"].values()))
57-
self.assertEqual(
58-
"AUTOMATICALLY_LINKED_TO", path["path"][0]["relationship"]
59-
)
59+
self.assertEqual("AUTOMATICALLY_LINKED_TO", path["path"][0]["relationship"])
6060

6161
@patch(
6262
"application.utils.gap_analysis.build_direct_cre_overlap_map_analysis",
6363
return_value={"result": {"x": {}}},
6464
)
65-
def test_backfill_refresh_recomputes_cached_pairs(
66-
self, build_mock: Mock
67-
) -> None:
65+
def test_backfill_refresh_recomputes_cached_pairs(self, build_mock: Mock) -> None:
6866
collection = Mock()
6967
collection.standards.return_value = ["ASVS"]
7068

application/tests/pci_dss_parser_test.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ def test_resolve_cre_falls_back_to_bridge_standard(self) -> None:
4545
prompt.get_id_of_most_similar_cre_paginated.return_value = (None, None)
4646
bridge_cre = defs.CRE(id="999-001", name="Bridge CRE", description="")
4747

48-
with patch.object(
48+
with patch.object(pci_mod, "PCI_BRIDGE_STANDARDS", ("S1", "S2")), patch.object(
4949
pci_mod, "best_cre_via_bridge_standard", side_effect=[None, bridge_cre]
5050
) as bridge_mock:
5151
cre = resolve_cre_for_pci_control(prompt, cache, [0.1, 0.2])

application/tests/secure_headers_parser_test.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,7 @@ class Repo:
4848
name="Secure Headers",
4949
hyperlink="https://example.com/foo/bar",
5050
section="headerAsection",
51-
links=[
52-
defs.Link(
53-
document=cre, ltype=defs.LinkTypes.AutomaticallyLinkedTo
54-
)
55-
],
51+
links=[defs.Link(document=cre, ltype=defs.LinkTypes.AutomaticallyLinkedTo)],
5652
tags=[
5753
"family:guidance",
5854
"subtype:cheatsheet",
@@ -69,7 +65,9 @@ class Repo:
6965
self.assertCountEqual(expected.todict(), nodes[0].todict())
7066

7167
@patch.object(git, "clone")
72-
def test_register_headers_creates_one_entry_per_opencre_link(self, mock_clone) -> None:
68+
def test_register_headers_creates_one_entry_per_opencre_link(
69+
self, mock_clone
70+
) -> None:
7371
class Repo:
7472
working_dir = ""
7573

@@ -96,10 +94,12 @@ class Repo:
9694
)
9795
nodes = entries.results[secure_headers.SecureHeaders().name]
9896
self.assertEqual(2, len(nodes))
99-
self.assertEqual({"First", "Second"}, {node.section for node in nodes})
10097
self.assertEqual(
101-
{"636-347", "743-110"},
102-
{node.links[0].document.id for node in nodes},
98+
{
99+
"First": "636-347",
100+
"Second": "743-110",
101+
},
102+
{node.section: node.links[0].document.id for node in nodes},
103103
)
104104

105105
@patch.object(git, "clone")

application/utils/external_project_parsers/parsers/pci_dss.py

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,50 @@
1717
logger = logging.getLogger(__name__)
1818
logger.setLevel(logging.INFO)
1919

20-
PCI_DSS_CRE_SIMILARITY_THRESHOLDS: tuple[float, ...] = tuple(
21-
float(part.strip())
22-
for part in os.environ.get(
23-
"PCI_DSS_CRE_SIMILARITY_THRESHOLDS", "0.55,0.45,0.35"
24-
).split(",")
25-
if part.strip()
20+
_DEFAULT_PCI_DSS_CRE_SIMILARITY_THRESHOLDS = (0.55, 0.45, 0.35)
21+
_DEFAULT_PCI_BRIDGE_STANDARDS = ("NIST 800-53 v5", "ISO 27001", "ASVS", "CWE")
22+
_DEFAULT_PCI_BRIDGE_MIN_SIMILARITY = 0.4
23+
24+
25+
def _parse_float_env(name: str, default: float) -> float:
26+
raw = os.environ.get(name, "").strip()
27+
if not raw:
28+
return default
29+
try:
30+
return float(raw)
31+
except ValueError:
32+
logger.warning("Invalid %s=%r; using default %s", name, raw, default)
33+
return default
34+
35+
36+
def _parse_float_tuple_env(name: str, default: tuple[float, ...]) -> tuple[float, ...]:
37+
raw = os.environ.get(name, "").strip()
38+
if not raw:
39+
return default
40+
try:
41+
values = tuple(float(part.strip()) for part in raw.split(",") if part.strip())
42+
except ValueError:
43+
logger.warning("Invalid %s=%r; using defaults %s", name, raw, default)
44+
return default
45+
return values or default
46+
47+
48+
def _parse_str_tuple_env(name: str, default: tuple[str, ...]) -> tuple[str, ...]:
49+
raw = os.environ.get(name, "").strip()
50+
if not raw:
51+
return default
52+
values = tuple(part.strip() for part in raw.split(",") if part.strip())
53+
return values or default
54+
55+
56+
PCI_DSS_CRE_SIMILARITY_THRESHOLDS = _parse_float_tuple_env(
57+
"PCI_DSS_CRE_SIMILARITY_THRESHOLDS", _DEFAULT_PCI_DSS_CRE_SIMILARITY_THRESHOLDS
2658
)
27-
PCI_BRIDGE_STANDARDS: tuple[str, ...] = tuple(
28-
part.strip()
29-
for part in os.environ.get(
30-
"PCI_DSS_BRIDGE_STANDARDS",
31-
"NIST 800-53 v5,ISO 27001,ASVS,CWE",
32-
).split(",")
33-
if part.strip()
59+
PCI_BRIDGE_STANDARDS = _parse_str_tuple_env(
60+
"PCI_DSS_BRIDGE_STANDARDS", _DEFAULT_PCI_BRIDGE_STANDARDS
3461
)
35-
PCI_BRIDGE_MIN_SIMILARITY = float(
36-
os.environ.get("PCI_DSS_BRIDGE_MIN_SIMILARITY", "0.4")
62+
PCI_BRIDGE_MIN_SIMILARITY = _parse_float_env(
63+
"PCI_DSS_BRIDGE_MIN_SIMILARITY", _DEFAULT_PCI_BRIDGE_MIN_SIMILARITY
3764
)
3865

3966

@@ -119,9 +146,7 @@ def resolve_cre_for_pci_control(
119146
return cre
120147

121148
for standard_name in PCI_BRIDGE_STANDARDS:
122-
cre = best_cre_via_bridge_standard(
123-
cache, control_embedding, standard_name
124-
)
149+
cre = best_cre_via_bridge_standard(cache, control_embedding, standard_name)
125150
if cre:
126151
return cre
127152

application/utils/external_project_parsers/parsers/secure_headers.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,7 @@ def resolve_cre_external_id(
6969
return cres, candidate
7070
raise SecureHeadersLinkError(
7171
f"Secure Headers markdown references unknown CRE id {external_id!r}"
72-
+ (
73-
f" (also tried remap {remapped!r})"
74-
if remapped
75-
else ""
76-
)
72+
+ (f" (also tried remap {remapped!r})" if remapped else "")
7773
)
7874

7975
def parse(self, cache: db.Node_collection, ph: prompt_client.PromptHandler):
@@ -119,9 +115,7 @@ def register_headers(self, cache: db.Node_collection, repo, file_path, repo_path
119115
queries = parse_qs(parsed.query)
120116
section = queries.get("section")
121117
link = queries.get("link")
122-
cres, _resolved_id = self.resolve_cre_external_id(
123-
cache, creID
124-
)
118+
cres, _resolved_id = self.resolve_cre_external_id(cache, creID)
125119
cs = self.entry(
126120
section=section[0] if section else "",
127121
hyperlink=link[0] if link else "",

scripts/compute_pci_dss_cre_mappings.py

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,9 @@
5757
def _configure_llm_env() -> None:
5858
embed_model = os.environ.get("CRE_EMBED_MODEL")
5959
if not embed_model:
60-
vertex_embed = os.environ.get("VERTEX_EMBED_CONTENT_MODEL", "gemini-embedding-001")
60+
vertex_embed = os.environ.get(
61+
"VERTEX_EMBED_CONTENT_MODEL", "gemini-embedding-001"
62+
)
6163
os.environ["CRE_EMBED_MODEL"] = f"gemini/{vertex_embed}"
6264
os.environ.setdefault("CRE_EMBED_EXPECTED_DIM", "3072")
6365
os.environ.setdefault("CRE_VALIDATE_EMBED_DIM_ON_INIT", "0")
@@ -117,7 +119,9 @@ def compute_mappings(
117119
for index, row in enumerate(rows[:total], start=1):
118120
section_id = str(row.get("PCI DSS ID", "")).strip()
119121
section = str(row.get("Defined Approach Requirements", "")).strip()
120-
description = str(row.get("Requirement Description", "") or row.get("Guidance", "")).strip()
122+
description = str(
123+
row.get("Requirement Description", "") or row.get("Guidance", "")
124+
).strip()
121125
control = defs.Standard(
122126
name="PCI DSS",
123127
sectionID=section_id,
@@ -154,14 +158,18 @@ def main() -> int:
154158
parser = argparse.ArgumentParser(description="Compute PCI DSS → CRE mappings")
155159
parser.add_argument(
156160
"--cache-file",
157-
default=os.environ.get("CRE_CACHE_FILE", os.path.join(_REPO_ROOT, "standards_cache.sqlite")),
161+
default=os.environ.get(
162+
"CRE_CACHE_FILE", os.path.join(_REPO_ROOT, "standards_cache.sqlite")
163+
),
158164
)
159165
parser.add_argument(
160166
"--output",
161167
default=os.path.join(_REPO_ROOT, "data", "pci_dss_cre_mappings.json"),
162168
)
163169
parser.add_argument("--sheet-url", default=PCI_SHEET_CSV_URL)
164-
parser.add_argument("--limit", type=int, default=None, help="process only first N controls")
170+
parser.add_argument(
171+
"--limit", type=int, default=None, help="process only first N controls"
172+
)
165173
args = parser.parse_args()
166174

167175
logging.basicConfig(level=logging.INFO, format="%(levelname)s:%(name)s:%(message)s")

0 commit comments

Comments
 (0)