Skip to content

chore: restack legacy manifest rename on hardened main - #143

Closed
seonghobae wants to merge 20 commits into
chore/rename-deploy-manifestfrom
main
Closed

chore: restack legacy manifest rename on hardened main#143
seonghobae wants to merge 20 commits into
chore/rename-deploy-manifestfrom
main

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Internal reconstruction step for #75. This merges the current protected main into the historical path-only rename branch so its one-line commercial-readiness path change and manifest rename can be re-evaluated against the now-hardened Kubernetes manifest from #137. This does not target protected main and is not a release/merge-readiness claim.


Devin Review

seonghobae and others added 20 commits August 16, 2026 20:49
Update the immutable github/codeql-action/upload-sarif pin from the official v4.37.5 commit to the official v4.37.6 commit. Exact-head CI, Security Scan, and SAST Semgrep succeeded.
- src/lib.rs: fix cosmetic json! spacing ("model":model -> "model": model)
  flagged by Devin, so it reads consistently with the adjacent
  "orchestration_mode": "auto" line. rustfmt doesn't reformat inside
  serde_json::json! macro bodies, so this was silently inconsistent.

- tests/adaptive_orchestrator_default.rs: per CodeRabbit's suggested diff,
  the uniqueness assertion now scans the *entire* source file for
  "orchestration_mode": "auto" and asserts both that there's exactly one
  occurrence and that it falls within soc_llm_chat_body's bounds --
  previously it only counted matches inside the already-extracted
  function_source slice, so a second occurrence added to an unrelated
  function elsewhere in src/lib.rs would have passed undetected.

- docs/adr/0010-adaptive-contextual-orchestrator-default.md: added a
  literature-to-decision mapping table connecting each cited paper's
  mechanism and reported ablation/metric (TRINITY's role separation and
  LiveCodeBench pass@1; Conductor's recursive test-time scaling and
  LiveCodeBench/GPQA-Diamond records; Fugu's query-adaptive scaffolding
  and SWE-Bench Pro/Terminal-Bench state-of-the-art; Omidvar & Akhlaghi's
  cost-aware Pareto routing) to the specific ADR decision item it
  grounds, per CodeRabbit's request and AGENTS.md's research-grounding
  convention. Did not add PDF copies to docs/papers/: all four are
  already cited with arXiv links and now have per-paper summaries in the
  mapping table, satisfying AGENTS.md's explicit fallback ("attach the
  PDF only when redistribution is permissible; otherwise cite + link +
  summarize") without taking on individual per-paper license review for
  redistribution.

cargo fmt --check, cargo test --locked --workspace, and cargo clippy
--locked --workspace --all-targets -D warnings are all clean (aside
from the pre-existing, environment-specific
binary_serves_then_shuts_down_on_sigterm local-sandbox flake unrelated
to this change).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Bumps [github/codeql-action/upload-sarif](https://github.com/github/codeql-action) from 4.37.6 to 4.37.7.
- [Release notes](https://github.com/github/codeql-action/releases)
- [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md)
- [Commits](github/codeql-action@5595cca...ff2f1c6)

---
updated-dependencies:
- dependency-name: github/codeql-action/upload-sarif
  dependency-version: 4.37.7
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [futures-util](https://github.com/rust-lang/futures-rs) from 0.3.33 to 0.3.34.
- [Release notes](https://github.com/rust-lang/futures-rs/releases)
- [Changelog](https://github.com/rust-lang/futures-rs/blob/main/CHANGELOG.md)
- [Commits](rust-lang/futures-rs@0.3.33...0.3.34)

---
updated-dependencies:
- dependency-name: futures-util
  dependency-version: 0.3.34
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Seongho Bae <seongho@kw.ac.kr>
…trator-default

feat(ai): delegate SOC analysis to adaptive orchestration
The gateway pod never calls the Kubernetes API, so the default
ServiceAccount token should not be automounted into the container
filesystem. This is the one Trivy Kubernetes Security (KSV) misconfig
pattern the prior deployment-hardening pass (KSV-0020/KSV-0021/
KSV-0125, PR #14) left unaddressed.
tests/binary.rs::binary_serves_then_shuts_down_on_sigterm was failing
CI on this branch (job 99248003533): the gateway exited with raw
signal 15 instead of code 0.

Root cause: shutdown_signal() was an async fn, so
tokio::signal::unix::signal() (which installs the OS-level handler)
only ran once the future was first polled inside
axum::serve(...).with_graceful_shutdown(shutdown) -- which happens
after run_from_env has already bound the listener and printed the
readiness line. A SIGTERM delivered in that window (exactly what the
e2e test, and any container runtime with a short
terminationGracePeriodSeconds, does right after startup) fell through
to the default disposition and killed the process instead of
triggering a graceful shutdown.

Fix: split registration from waiting. install_shutdown_signal() calls
tokio::signal::unix::signal() synchronously and returns only the
subsequent .recv() as a future, so main() installs the handler before
run_from_env runs at all.

Verified: `cargo test -p waf-ids-ai-soc --test binary` was reliably
failing before this change and passes 5/5 after.
Devin Review flagged (PR #132, src/main.rs:29-32) that the previous
commit only fixed the readiness/signal-registration race on the Unix
path: the non-Unix install_shutdown_signal() still wrapped
tokio::signal::ctrl_c() in an async block, so registration stayed
lazy and an immediate Ctrl-C could still kill the process before the
handler was installed.

tokio::signal::windows::ctrl_c() (unlike the cross-platform
tokio::signal::ctrl_c() convenience fn) registers synchronously and
returns a CtrlC handle, exactly mirroring unix::signal()/Signal, so
apply the same split-registration-from-waiting fix there.

Not covered by tests/binary.rs (its Windows test only exercises a
forced kill, not graceful Ctrl-C), and this sandbox has no Windows
cross-compiler to build/run against; verified the API shape directly
against the vendored tokio 1.53.1 source
(signal/windows.rs: `pub fn ctrl_c() -> io::Result<CtrlC>`,
`pub async fn recv(&mut self) -> Option<()>`), which matches the
already-used unix::Signal shape exactly.
fix(deploy): disable service account token automount (KSV-0036)
…s (KEV) catalog (#133)

* feat(threat-intel): add CISA KEV catalog ingestion adapter

Adds a new threat-intel adapter alongside the existing STIX/MISP/TAXII/
OpenCTI family: POST /api/threat-intel/cisa-kev fetches the CISA Known
Exploited Vulnerabilities catalog (a single well-known public JSON feed,
https://www.cisa.gov/known-exploited-vulnerabilities-catalog) and upserts
one `cve`-typed ThreatIndicator per entry, keyed by CVE ID, severity
escalated to Critical when CISA has tied the CVE to a known ransomware
campaign (High otherwise — mere inclusion in the catalog already signals
CISA-confirmed active exploitation).

- src/kev_import.rs: pure parser (`parse_kev_document`), mirroring the
  misp_import/opencti_import/stix_import module shape exactly. Accepts
  the real catalog shape (`{"vulnerabilities": [...]}`) or a bare array.
  KEV entries carry no IP/domain/URL/hash observable, so `dnsbl` stays
  empty (kept only for ThreatFeedImport parity).
- src/lib.rs: `import_kev_feed` handler follows the phishing-database
  fetch-by-URL pattern (not the MISP/STIX/OpenCTI paste-a-document
  pattern) since KEV has one canonical, stable, publicly known URL that
  every consumer wants pulled automatically rather than hand-relayed.
  Reuses the existing `fetch_text_feed`/`apply_threat_feed_import`
  plumbing untouched. `validate_http_url` gained a third `allowed_hosts`
  parameter (was hardcoded to the phishing-database allowlist) so KEV
  gets its own `www.cisa.gov`-only default, matching the SSRF-safety
  posture phishing-database already has (opt-out via
  `allow_non_default_hosts`); all other call sites updated.
- crates/waf-ids-core: new buyer-evidence-manifest endpoint entry.
- docs/architecture.md, docs/security/compliance-mapping.md: documented
  alongside the other threat-intel adapters.

Deliberately not adding a cargo-fuzz target: none of the five existing
JSON-import adapters (misp_import, opencti_import, stix_import,
suricata_eve, coraza_audit) have one today — CLAUDE.md's fuzzed-surface
list (request scorer, state deserializer, admin-token parser, DNSBL zone
export) doesn't cover this adapter family. Matched the existing
convention instead: a `parse_never_panics_on_arbitrary_text` unit test,
same as its siblings.

Verified: cargo fmt --check, cargo test --locked --workspace (all pass
except the pre-existing root-sandbox-only `load_surfaces_state_rewrite_failures`
flake tracked by PR #93, unrelated to this change), cargo clippy
--locked --workspace --all-targets -- -D warnings.

* fix(waf): never content-match CVE threat indicators in scoring

Devin Review flagged (PR #133): every KEV-imported CVE indicator was
entering score_request's generic substring matcher at High/Critical
severity. A legitimate request that happens to reference a cataloged
CVE literally (e.g. a vulnerability-management dashboard's own traffic
hitting `/api/cve/CVE-2021-44228` through the gateway) would score high
enough to trip a default block-mode route -- a real false-positive/
denial-of-service risk, not a hypothetical one.

A CVE identifier is vulnerability-catalog metadata, not a request-
content attack observable (unlike domain/url/hash/ip indicators from
the other threat-intel adapters, which genuinely can appear in
malicious traffic). Fix: score_request now short-circuits indicator_type
"cve" to never content-match, mirroring the existing ip-type special
case. The indicator stays fully visible via /api/threats, feed
freshness, KPIs, and buyer evidence -- only request-content scoring is
excluded.

Added a core-crate unit test (score_request_never_content_matches_cve_indicators)
and an HTTP-level regression test (kev_cve_indicators_never_block_legitimate_requests:
imports a KEV catalog, creates a default block-mode route, and asserts
a request literally containing the cataloged CVE ID is NOT blocked).
Verified both fail without the fix and pass with it.

* fix(shutdown): port SIGTERM/Ctrl-C race fix from PR #132

This branch forked from main before #132's shutdown-handler fix
merged, so its copy of src/main.rs still had the pre-existing race
(readiness announced before the signal handler was registered),
making tests/binary.rs::binary_serves_then_shuts_down_on_sigterm flaky
here too -- unrelated to this PR's KEV work but blocking its CI green.
Ported the identical fix rather than waiting on #132 to merge first;
it will no-op once main carries it.

* docs(research): ground CISA KEV severity policy in EPSS/BOD literature

Devin Review flagged (PR #133) that this substantive feature PR was
missing the research grounding AGENTS.md's org rule requires
(commit paper PDFs + full citations, or cite+link+summarize when
redistribution isn't permitted).

Added a "Further reading" section to docs/architecture.md (matching
docs/fuzzing.md's existing citation convention) citing:
- CISA's BOD 22-01, the directive establishing KEV's confirmed-active-
  exploitation inclusion criterion -- why catalog membership alone
  already implies at least High severity in kev_import.rs.
- Jacobs et al. (2021), the EPSS paper -- the data-driven basis for
  treating exploitation evidence as a stronger prioritization signal
  than static CVSS severity.
- Shimizu & Hashimoto (2025, arXiv:2506.01220, CC BY 4.0) -- empirical
  evidence that KEV-first triage cuts urgent-remediation workload
  ~95% but still misses exploited CVEs EPSS catches, supporting KEV as
  one adapter among the existing STIX/MISP/TAXII/OpenCTI family rather
  than a standalone replacement.

Committed the arXiv PDF (CC BY 4.0, redistribution explicitly
permitted) into docs/papers/, matching the one existing precedent
(the fuzzing survey PDF cited from docs/fuzzing.md). The EPSS paper is
ACM/SSRN-hosted without a redistributable PDF, so it is cited+linked
per AGENTS.md's explicit fallback instead of attached.

* fix(security): don't route request-supplied kev_url into the fetch by default

CodeQL flagged (PR #133, critical severity, rust/request-forgery at
src/lib.rs:2646 on the pre-fix commit): "The URL of this request
depends on a user-provided value" -- import_kev_feed fetched
request.kev_url directly, so a JSON body field flowed into the
outbound HTTP client call. validate_kev_import_request already
allowlists the host (www.cisa.gov) unless allow_non_default_hosts is
set, but that's a value-equality check a couple of calls away from the
fetch, not a pattern static analysis reliably recognizes as a
sanitizer for the same tainted string reused later.

Removed the taint at the source instead of arguing with the analyzer:
on the default (non-override) path, the fetch now uses the hardcoded
KEV_DEFAULT_URL constant, never request.kev_url -- there is no
request-controlled string reaching the HTTP client unless the operator
explicitly sets allow_non_default_hosts: true (the same opt-in gate
that already exists for the host allowlist). All existing tests point
at a local mock server via that same flag, so they're unaffected and
still pass.

Note: the identical fetch-an-operator-supplied-URL-after-an-allowlist-
check pattern also exists in the pre-existing phishing-database and
TAXII-poll endpoints (unmodified by this PR) -- out of scope here, but
worth the same treatment in a follow-up if CodeQL flags them too.

* fix(kev): stop validating kev_url when it won't be used

Devin Review flagged (PR #133): with allow_non_default_hosts false,
validate_kev_import_request still checked kev_url against the CISA
host allowlist even though import_kev_feed (65d60a9) now always
fetches the hardcoded default in that mode -- so a validated
same-host custom path was silently discarded, and the validation
itself was misleading (accept-then-ignore).

Chose Devin's second suggested option over restoring same-host
customization: honoring a validated-but-still-request-controlled URL
on the default path is exactly the pattern that triggered the
original CodeQL SSRF alert, so reintroducing it isn't a real fix.
Instead, kev_url is now only validated when allow_non_default_hosts
is true (the one mode where it's actually fetched) -- and once
opted in, any well-formed http(s) URL is accepted with no host
restriction, matching how that same flag already works for
phishing-database and TAXII. KEV_ALLOWED_HOSTS is removed as dead
code; there is no longer a partial-trust "same host, no opt-in"
tier to enforce.

Replaced the now-invalid kev_feed_import_rejects_disallowed_host_by_default
HTTP test (there is no more "disallowed host" rejection under the new
contract) with a validate_kev_import_request unit test covering both
modes directly.

* fix(security): remove kev_url from the request contract entirely

CodeQL's critical SSRF alert (rust/request-forgery) was still firing
on the previous fix (65d60a9): gating the tainted request.kev_url
behind an allow_non_default_hosts runtime flag doesn't register as a
sanitizer to CodeQL's dataflow analysis -- the string still
originates from the HTTP request body and still reaches
fetch_text_feed on that code path, so the alert (rightly, from a
pure taint-tracking standpoint) persisted.

Rather than keep trying to convince the analyzer a runtime guard is
safe, removed the taint source outright: kev_url and
allow_non_default_hosts are gone from KevImportRequest entirely.
import_kev_feed always fetches AppState::kev_catalog_url, which is
deployment-time config only -- set via the new KEV_CATALOG_URL env
var (validated at startup, alongside run_from_env's other env
parsing) or AppState::with_kev_catalog_url() in tests -- never
sourced from a client request. There is now no code path in this
handler where request-supplied data reaches an outbound fetch at
all.

This is a stronger fix than the previous one on the merits too, not
just for the analyzer: KEV genuinely has one canonical, stable,
government-published URL, so per-request override was never load-
bearing functionality, just a testing convenience that's now served
by the AppState builder instead.

Updated the three affected tests to point AppState::kev_catalog_url
at a local mock server instead of the request body, and replaced the
now-obsolete kev_url validation test with a plain feed-metadata
validation test. CLAUDE.md's Runtime Configuration section documents
the new env var.

* fix(kev): decouple KEV catalog fetch from the shared request-URL sink

fetch_text_feed's url parameter is fed by phishing-database's
request-supplied domain_url/ip_url (a pre-existing, admin-gated
"fetch from an operator-chosen URL" design on main). Routing the KEV
catalog pull through that same shared function -- confirmed via diff
against origin/main that fetch_text_feed/validate_http_url were
otherwise byte-identical -- was enough for CodeQL's rust/request-forgery
query to keep flagging the shared sink as newly touched by this PR,
even after KEV_CATALOG_URL was made config-only.

Give KEV its own fetch_kev_catalog function (same pattern already used
by fetch_taxii_objects) so the config-only path never shares a
function with the request-URL adapters, and revert validate_http_url
to its original two-argument form now that KEV no longer needs a
custom allowed-hosts list.

Also add KEV_CATALOG_URL to clear_run_env: Devin Review flagged that
an inherited value would leak across run_from_env tests, since the
list omitted it while validating every other run_from_env env var.

* fix(kev): restrict KEV_CATALOG_URL to CISA's own host

Devin Review flagged that KEV_CATALOG_URL had no host allowlist, so an
unintended inherited or misconfigured value could redirect this
privileged catalog fetch -- a real SSRF sub-pattern (env-var-sourced
URLs are externally influenceable via container/orchestrator
inheritance, not just via request bodies), which lines up with why
CodeQL kept flagging feed_http.get(url) here even after the previous
commit fully decoupled the fetch from any request-derived data.

Add validate_kev_catalog_url, enforced both at startup (fail fast on a
bad env var) and again immediately before the fetch: the host must be
www.cisa.gov or loopback (loopback only so tests can point it at a
local mock server), with no override -- unlike the operator-URL
adapters, KEV has no legitimate reason to fetch from anywhere else.

* docs(kev): correct stale "internal mirror" language for the new host allowlist

Devin Review caught that AppState::with_kev_catalog_url's doc comment
still promised support for pointing KEV_CATALOG_URL at an internal
mirror, which the host-allowlist fix in 3a47ce8 now rejects at startup
-- a real contract mismatch, not just stale wording. There was never
an actual mirror requirement (that phrase was leftover illustrative
language from an earlier draft); update the builder's doc comment and
CLAUDE.md's Runtime Configuration entry to describe the real contract
(CISA's own host, or loopback for local test mocking) rather than
inventing a mirror-allowlist feature nobody asked for.

* fix: validate CVE ID syntax on KEV import; scope Windows cfg precisely

CodeRabbit review, both real:

- kev_import.rs: any non-empty cveID (e.g. "not-a-cve") was accepted
  and stored as a cve threat indicator. Add is_valid_cve_id, checking
  CVE.org's CVE-<4-digit year>-<4+ digit sequence> syntax
  case-insensitively; entries that fail it are now Skipped like a
  missing cveID already was. Panic-safe on arbitrary catalog text via
  str::get instead of direct slicing (added a multi-byte-boundary case
  to parse_never_panics_on_arbitrary_text to cover it).

- main.rs: install_shutdown_signal's second definition was gated on
  cfg(not(unix)), but its body calls tokio::signal::windows::ctrl_c,
  which only exists on Windows -- any other non-Unix target would fail
  to compile there. Scope the cfg to windows specifically.

* fix(threat-feeds): don't reap indicators still owned by another feed

Devin Review flagged a real bug in the feed-ownership reconciliation
logic introduced by an external merge (192ffba, authored outside this
session): apply_threat_feed_import removed every key the refreshing
feed had previously owned from the global threats collection
unconditionally, without checking whether another feed's ownership
record still claimed the same indicator_type+value+source key. Two
feeds sharing a key (e.g. the same CVE imported under a shared
`source`) meant refreshing either one made the indicator vanish from
enforcement until the other feed's next refresh happened to re-add it.

Track "still owned by some other feed" via the current (post-replace)
ownership table and only reap a dropped key when no feed claims it any
more. Also switches previous_keys from a Vec to a HashSet, which
incidentally fixes a second (lower-severity) finding on the same
commit: the per-threat previous_keys.contains() scan was O(n) per
retained item, made O(1) here.

Added feed_refresh_preserves_indicators_still_owned_by_another_feed,
which fails against the pre-fix logic (the shared indicator disappears
after the first feed's refresh) and passes with it -- verified by
reasoning through both code paths.

Not actioned (informational/acceptable trade-offs, not bugs):
- "Upgraded feeds retain withdrawn indicators": state persisted before
  this feature existed has no ownership history, so the first refresh
  per feed after upgrade can't retroactively reconcile pre-existing
  drift. Self-heals from each feed's first post-upgrade refresh
  onward; there's no safe way to backfill ownership for data imported
  before it was tracked.
- "DNSBL refresh remains append-only": reconciliation covers threats
  only, matching the scope of the original finding this responds to
  (KEV catalogs carry no DNSBL entries). Extending it to DNSBL entries
  for the other four adapters is a follow-up, not a regression here.

* fix(kev): fail closed without write creds

* fix(kev): reject mostly-unparsable catalogs; scope credential source to admin creds

Devin Review, both real:

- kev_import.rs: apply_threat_feed_import now reconciles (a refresh
  treats keys missing from the new snapshot as withdrawn and removes
  them). kev_material_from_value's only acceptance bar was "at least
  one usable cveID," so a catalog that's mostly unparsable -- a fetch
  truncated mid-transfer, a CISA response format regression -- would
  have been accepted and, via reconciliation, read as a mass
  withdrawal of still-exploited CVEs instead of the bad fetch it
  actually was. Require a real majority of entries to have parsed
  (skipped_entries <= threats.len()) before trusting a snapshot as
  authoritative. Adjusted skips_entries_with_malformed_cve_id's fixture
  to stay under the new bar and added
  rejects_catalog_where_most_entries_are_unparsable.

- credentials.rs: bootstrap_secrets set the registry-wide
  CredentialSource to File whenever ANY key -- including a file that
  supplies only kev_catalog_url -- came from the credentials file, even
  when ADMIN_TOKEN/ADMIN_TOKENS actually came from env. CredentialSource
  is documented and reported via HealthStatus/support bundle as admin
  credential provenance specifically, so this misreported security-
  relevant operational state. Track admin-credential file/env
  provenance independently of kev_catalog_url's, matching the
  documented contract. Added
  file_only_kev_catalog_url_does_not_misreport_env_admin_token_as_file_backed.

Not actioned in this commit -- flagging separately for the user's
scope decision: Devin also noted that apply_threat_feed_import's
reconciliation now applies uniformly to all five existing feed
adapters (STIX/MISP/TAXII/OpenCTI/phishing-database), not just KEV,
which changes their prior upsert-only semantics; anyone relying on
incremental/partial imports under a reused feed_id across the other
four adapters would now see earlier entries treated as withdrawn. This
is a cross-adapter behavior/policy question outside what a bug fix can
resolve unilaterally.

* fix(kev): remove runtime catalog URL override

* fix(kev): preserve operator indicators on feed refresh

* fix(kev): preserve operator-owned threat payloads

* fix(threat-feeds): report actual upserted count, not submitted count

Devin Review: apply_threat_feed_import's operator-ownership skip
(8b7954d) omits operator-owned threats from the upsert loop, but
ThreatFeedImportResult.upserted_threats still reported feed.threats.len()
-- the full submitted set, including entries that were skipped. Import
clients (including KEV's response) received an inflated success count
whenever a feed overlapped operator-managed data.

Count actual upserts in the loop and return that. ThreatFeedStatus.threat_count
is left as feed.threats.len() deliberately -- it represents catalog
membership (what this feed's snapshot claims), not applied mutations,
per Devin's suggested distinction.

Added import_result_excludes_operator_owned_threats_from_upserted_count.

---------

Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: OpenAI Codex <codex@openai.com>
* test(deploy): reject shipped admin credentials

* fix(deploy): require externally provisioned admin secret

* docs(deploy): document external admin secret lifecycle

* docs(security): trace external secret boundary

* docs: establish security changelog

* test(deploy): reproduce admin secret bootstrap gaps

* fix(deploy): reject duplicate admin token entries

* docs(deploy): bootstrap namespace before secret sync

* docs(deploy): align manifest bootstrap guidance

* docs(security): trace bootstrap and current NIST status

* test(deploy): format manifest contract regressions

* test(deploy): fail closed on quoted admin token aliases

* test(deploy): reject commented admin token aliases

* docs(papers): add NIST SP 800-57 reference artifact

* test(deploy): reject escaped admin token fallbacks

* test(deploy): satisfy strict clippy without weakening manifest checks

---------

Co-authored-by: OpenAI Codex <codex@openai.com>
@seonghobae seonghobae closed this Sep 1, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 5 potential issues.

Devin Review

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: Research attachment rule is satisfied

The PR attaches permitted research PDFs and links restricted sources without redistribution. The new KEV and orchestration work follows the repository rule.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/lib.rs
Comment on lines +2716 to +2722
let previous_keys: HashSet<_> = replace_threat_feed_ownership(
&mut data.threat_feed_ownership,
feed.feed_id.clone(),
threat_keys,
)
.into_iter()
.collect();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Incremental imports erase earlier indicators

apply_threat_feed_import treats every document as a complete snapshot. Standalone STIX/MISP imports and TAXII added_after polls delete indicators from earlier requests.

Prompt for agents
Threat ownership reconciliation in src/lib.rs::apply_threat_feed_import assumes every call supplies a complete snapshot. However, import_stix_document and import_misp_document accept standalone documents under stable default feed IDs, and poll_taxii_collection explicitly supports added_after incremental polls. A later partial import therefore replaces the prior ownership set and reaps valid older threats. Distinguish snapshot replacement from additive or incremental ingestion in the import contract. Only reconcile removals for adapters that provide a verified complete snapshot, while merging ownership for standalone and incremental imports. Add regression tests covering sequential standalone STIX/MISP imports and TAXII added_after polls.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +14 to +15
#[serde(default)]
pub operator_threat_keys: Vec<ThreatIndicatorKey>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Upgrades lose existing threat ownership

Legacy state loads both ownership lists empty. Feed refreshes leave withdrawn entries active and can overwrite or later delete manually managed entries.

Prompt for agents
Existing persisted AppData has threats and threat_feeds but lacks operator_threat_keys and threat_feed_ownership. Serde defaults both new fields to empty, so ownership cannot be inferred after upgrade. The first refresh records only the current snapshot and cannot remove already-withdrawn legacy feed threats. Existing operator-created threats are also unmarked, allowing a matching feed import to overwrite their payload and a later refresh to delete them. Add an explicit persisted-state migration or a backward-compatible ownership model that preserves legacy operator data and safely establishes feed ownership before enabling destructive reconciliation. Cover loading a pre-change state file followed by multiple feed refreshes.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/lib.rs
Comment on lines 2751 to 2752
for entry in feed.dnsbl.iter().cloned() {
upsert_dnsbl(&mut data.dnsbl, entry);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 DNSBL refresh remains additive

apply_threat_feed_import reconciles threats but only upserts DNSBL rows. Feed status can describe a new snapshot while withdrawn DNSBL entries remain active.

(Refers to this code)

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +893 to +900
} else if kind == "cve" {
// A CVE identifier is vulnerability-catalog metadata (e.g. from a
// CISA KEV import), not a request-content observable: it can
// legitimately appear in a vulnerability-management or security
// tool's own traffic (`/api/cve/CVE-2021-44228`), so it must never
// drive content-substring scoring. It stays visible via the
// threat-indicator, feed-freshness, and buyer-evidence APIs.
false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: CVE metadata stays outside enforcement

score_request excludes cve indicators from content matching. KEV catalog entries remain evidence metadata without blocking requests that mention a CVE.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants