feat(cti): ingest HABIT external assistant URL and map to phishing (T1566.002)#22286
feat(cti): ingest HABIT external assistant URL and map to phishing (T1566.002)#22286BrianCLong wants to merge 1 commit intomainfrom
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 11 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces a new CTI item for the HABIT conference demo and adds a corresponding mapping rule in the TTP mapper to detect external assistant URL solicitation. The feedback identifies a critical key mismatch between the ingest data and the mapper, case-sensitivity issues in the mapping logic, and fragile hardcoded indices in the test suite.
| ] | ||
| }, | ||
| { | ||
| "url": "https://www.habitsimulation.xyz/scotobot", |
There was a problem hiding this comment.
The key url is used here, but the TTP mapper in services/ttp_mapper/src/mapper.py (line 72) expects source_url. This discrepancy will likely cause a KeyError during the mapping phase if the item is passed directly to the mapper.
| "url": "https://www.habitsimulation.xyz/scotobot", | |
| "source_url": "https://www.habitsimulation.xyz/scotobot", |
| if ( | ||
| "human-ai persuasion" in full_text | ||
| or "external domain solicitation" in full_text | ||
| or "conference demos" in full_text | ||
| ): |
There was a problem hiding this comment.
The current matching logic has two issues:
- Case Sensitivity: The keywords are lowercase, but the source data in
ingest.pyuses Title Case (e.g., "Conference demos"), which will fail to match in a standard string comparison. - Keyword Mismatch: The keyword
"human-ai persuasion"does not match the text in the ingested item, which uses"Operator-agent persuasion".
Using .lower() and aligning the keywords with the actual data will make the detection robust.
| if ( | |
| "human-ai persuasion" in full_text | |
| or "external domain solicitation" in full_text | |
| or "conference demos" in full_text | |
| ): | |
| if any(kw in full_text.lower() for kw in [ | |
| "operator-agent persuasion", | |
| "external domain solicitation", | |
| "conference demos" | |
| ]): |
| assert any(m["control"] == "Repo Hardening (Branch Protection)" for m in ai_mapping["mappings"]) | ||
|
|
||
| # Check conference external URL social-engineering surface | ||
| habit_mapping = mappings[5] |
There was a problem hiding this comment.
Accessing the mapping by a hardcoded index (mappings[5]) is fragile. The test will break if the order of items in normalize_items() is changed or if new items are added earlier in the list. Searching by source_url is more maintainable.
| habit_mapping = mappings[5] | |
| habit_mapping = next(m for m in mappings if m["source_url"] == "https://www.habitsimulation.xyz/scotobot") |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 481e56624c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| "human-ai persuasion" in full_text | ||
| or "external domain solicitation" in full_text | ||
| or "conference demos" in full_text |
There was a problem hiding this comment.
Require URL-specific signal in new phishing rule
The new rule classifies any item containing "conference demos" as T1566.002, even when there is no phishing or external-link solicitation context. In map_item, full_text is built from title/claims only, so benign CTI like conference recaps will now be mislabeled as phishing (I verified with a sample title "Conference demos for SOC tooling"). This introduces systematic false positives in ATT&CK mapping outputs; the condition should require a URL solicitation indicator (or a conjunction of terms) instead of this standalone generic phrase.
Useful? React with 👍 / 👎.
## Summary Rebuilds the golden-main merge train from a clean `main` base and converges the currently mergeable PR set into one replacement branch. This branch absorbs: - #22296 - #22279 - #22281 - #22282 - #22283 - #22284 - #22285 - #22295 - #22297 - #22286 - #22291 - #22280 - #22277 - unique non-conflicting surfaces from #22241 - the admissibility/CACert/failure-demo runtime lane from #22314 This branch supersedes: - #22298 as the contaminated/conflicting convergence branch - #22277 as a standalone merge vehicle - #22241 as the broad mixed-purpose convergence vehicle once remaining review is complete - #22314 as the standalone admissibility lane now folded into the golden path This branch intentionally excludes: - #22292 because it targets `merge-surge/staging`, not `main` ## Conflict policy used while absorbing #22241 When merging `#22241` on top of the cleaned train, the following files conflicted and were resolved in favor of the current train versions so the newer focused CI/governance repairs remain authoritative: - `.github/ci/required-checks.json` - `.github/workflows/drift-sentinel.yml` - `.github/workflows/pr-gate.yml` - `docs/ci/REQUIRED_CHECKS_POLICY.yml` - `pnpm-lock.yaml` - `scripts/ci/check_branch_protection_drift.mjs` - `scripts/ci/validate_workflows.mjs` All other `#22241` changes merged on top of the train. ## Mapping Change Summary This convergence branch updates workflow, schema, and governance contracts that control merge eligibility, admissibility evidence, and deterministic trust artifacts. ## Diff - Added admissibility/evidence/CACert surfaces including `packages/evidence/schemas/decision_trace.schema.json` - Tightened golden-lane workflow policy and drift handling in `.github/workflows/_policy-enforcer.yml`, `.github/workflows/execution-graph-reconciliation.yml`, `.github/workflows/post-ga-hardening-enforcement.yml`, `.github/workflows/merge-surge.yml`, `.github/workflows/control-plane-drift.yml` - Realigned governance state in `governance/pilot-ci-policy.json` and `governance/branch-protection.json` - Repaired deterministic reconciliation verification in `scripts/ci/verify_execution_graph_reconciliation.mjs` and `scripts/ci/drift-sentinel.mjs` ## Justification The repo needed one mergeable replacement lane that restores deterministic governance checks, folds the admissibility implementation into the golden path, and suppresses broken optional PR workflows that were blocking convergence without being canonical required checks. ## Impact - Canonical pilot checks remain `pr-gate / gate` and `drift-sentinel / enforce` - Merge-train branches no longer fail ordinary small-PR enforcement gates by construction - Optional broken workflows are narrowed to their owned surfaces so they stop contaminating this convergence lane and the immediate post-merge main push - Execution-graph reconciliation now accepts the repo’s canonical snake_case trust bundle fields ## Rollback Plan Revert commit `ce32b96c0f` from `merge-train/golden-main-20260331-final`, then rerun the prior golden-lane checks and restore the previous PR body. ## Backfill Plan After the lane is green, backfill the same workflow scoping and governance-contract repairs into any surviving PRs that still touch `.github/workflows/**` or governance surfaces, then close superseded PRs against `#22309`. ## Validation Evidence Local validation completed: - `node scripts/ci/drift-sentinel.mjs` - `ruby -e 'require "yaml"; ... YAML.load_file(...)'` over all edited workflow files - `jq . governance/pilot-ci-policy.json` - `jq . governance/branch-protection.json` - merge-marker scan over all edited files returned clean ## Notes - Live GitHub PR checks on the open PR set are being converged through this single branch instead of salvaging each broken lane independently. - I did not run the full local verification matrix in this session; this PR is intended to give the repo one clean convergence lane for CI and human review. - After this PR lands, the absorbed PRs should be closed as superseded. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com> Co-authored-by: Bot <bot@summit.ai> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Gemini CLI <gemini-cli@google.com>
|
Superseded by #22309, which is now merged into main. |
Motivation
Description
services/cti_ingest/src/ingest.pyto normalize the signal.services/ttp_mapper/src/mapper.pythat classifies this pattern asPhishing: Spearphishing Link (T1566.002)and recommendsURL Sandboxing and Domain Allowlistingas the control.services/cti_ingest/tests/test_pipeline.pyto validate the new ingest item and mapping path.{"task_id":"10429681155455260883","scope":"services/cti_ingest, services/ttp_mapper","files_changed":["services/cti_ingest/src/ingest.py","services/ttp_mapper/src/mapper.py","services/cti_ingest/tests/test_pipeline.py"],"change_type":"feat","confidence":0.88,"rollback_commit":"revert 0667948ad1"}
Testing
pytest -q services/cti_ingest/tests/test_pipeline.py, encounteredModuleNotFoundError: jsonschemaduring collection, installed it withpip install jsonschema, and re-ran tests successfully.3 passed in 1.01sfor the targeted test file.node scripts/check-boundaries.cjswhich returned✅ No boundary violations found.Codex Task