From 5680b9e05ecab155440a44dd2bafc4c621a42bc1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 02:25:58 +0000 Subject: [PATCH 1/3] =?UTF-8?q?feat(ecomops):=20knowledge=20base=20?= =?UTF-8?q?=E2=80=94=20shop=20policies=20for=20draft=20replies?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes a gap in what #19 shipped: the workflow accepted a `knowledge_lookup` but nothing implemented it, and the system prompt forbids answering from anything outside the "Dữ liệu" block. So shipping_fee and return_exchange — two of the seven intents — could never actually be answered; the bot could only promise to check. That's the safe failure mode, but it isn't a useful one. - knowledge.py — two interchangeable async lookups: * MagicKnowledgeLookup reads MagiC's Knowledge Hub (GET /api/v1/knowledge), so policies live alongside every other MagiC workload. * StaticKnowledgeLookup reads a YAML file, for shops running the pack against an in-memory core with no Hub. Entries carry two tags (`ecomops` + topic), and lookups require both. The Hub has no server-side tag filter, so without that a shipping question would get answered with the return policy whenever the keyword search overlapped — or worse, with unrelated org knowledge that merely shared a word. A Hub outage degrades to "no policy data" rather than dropping the message; the guardrails still catch anything the model invents to fill the gap. - sample_knowledge.yaml + seed_knowledge.py — Vietnamese starter KB (shipping fees, 7-day return policy, order statuses, product/complaint handling) and a seeder. Both the file and the seeder log loudly that this is sample data to be replaced with the shop's real policy, since wrong content here becomes a wrong answer to a customer. - CI: the install step now picks up every requirements.txt under connectors/ and packs/ instead of naming one file. Adding pyyaml for this feature would otherwise have gone missing on CI (it passed locally only because my venv happened to have it), and a future connector's deps would hit the same trap. Intentionally NOT included: the Nhanh.vn connector, which is the other Tuần 3-4 item. BUSINESS_MODEL.md and OPEN_CORE_STRATEGY.md both list the production Nhanh.vn connector as Commercial ("điểm khác biệt lớn"), and this is the public repo — that needs a deliberate decision, not a drive-by commit. Tests: 21 new (topic mapping, tag filtering, top_k, no-topic short-circuit, Hub-down degradation, malformed payloads, static YAML, seeding round-trip, plus a check that every topic in the sample YAML is reachable from some intent so the file can't drift into dead data). 191 total, all passing. --- .github/workflows/ci.yml | 10 +- packs/ecomops/README.md | 49 +++++++- packs/ecomops/__init__.py | 3 + packs/ecomops/example_worker.py | 5 + packs/ecomops/knowledge.py | 163 +++++++++++++++++++++++++ packs/ecomops/requirements.txt | 3 + packs/ecomops/sample_knowledge.yaml | 63 ++++++++++ packs/ecomops/seed_knowledge.py | 40 +++++++ packs/ecomops/tests/test_knowledge.py | 164 ++++++++++++++++++++++++++ 9 files changed, 495 insertions(+), 5 deletions(-) create mode 100644 packs/ecomops/knowledge.py create mode 100644 packs/ecomops/sample_knowledge.yaml create mode 100644 packs/ecomops/seed_knowledge.py create mode 100644 packs/ecomops/tests/test_knowledge.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee60a2d..0e6a300 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -118,9 +118,13 @@ jobs: # isn't part of this PR's diff). run: cd sdk/python && pip install -e ".[dev,connectors]" # NOSONAR python:S4830 python:S5445 - name: Install connector/pack deps - # google-auth is only needed by the Google Sheet connector, so it lives in - # that connector's requirements rather than the SDK's extras. - run: pip install -r connectors/google_sheet/requirements.txt # NOSONAR python:S4830 python:S5445 + # Deps that belong to a specific connector/pack rather than the SDK's + # extras — google-auth for the Google Sheet connector, pyyaml for the + # EcomOps knowledge base. Every requirements.txt under connectors/ and + # packs/ is installed, so adding a new one needs no CI change. + run: | # NOSONAR python:S4830 python:S5445 + find connectors packs -name requirements.txt -print0 \ + | xargs -0 -I{} pip install -r {} - name: Lint # Same ephemeral-CI-runner rationale as the installs above. run: pip install ruff && ruff check sdk/python connectors packs # NOSONAR python:S4830 python:S5445 diff --git a/packs/ecomops/README.md b/packs/ecomops/README.md index 83814e6..e111226 100644 --- a/packs/ecomops/README.md +++ b/packs/ecomops/README.md @@ -30,6 +30,7 @@ pytest packs/ecomops/tests | `intents.py` | Phân loại ý định: `RuleBasedIntentClassifier`, `LLMIntentClassifier`, `HybridIntentClassifier` | | `extraction.py` | Trích mã đơn hàng & số điện thoại từ tin nhắn | | `prompts.py` | Thư viện prompt tiếng Việt theo từng ý định | +| `knowledge.py` | Lấy chính sách shop (phí ship, đổi trả…) cho model soạn tin | | `guardrails.py` | Chặn câu trả lời rủi ro trước khi gửi cho khách | | `workflow.py` | `EcomOpsWorkflow` — pipeline nối tất cả lại | @@ -98,7 +99,45 @@ asyncio.run(main()) Khi nào chuyển nhân viên: khiếu nại (`complaint`), độ tin cậy thấp, guardrail chặn, hoặc soạn tin lỗi. -## 4. Demo end-to-end (Zalo + Google Sheet) +## 4. Knowledge Base (bắt buộc nếu muốn trả lời phí ship / đổi trả) + +System prompt bắt model **chỉ được trả lời dựa trên phần "Dữ liệu"**. Nếu không +cấp Knowledge Base thì câu hỏi về phí ship hay chính sách đổi trả sẽ không có gì +để trả lời — bot chỉ có thể nói sẽ kiểm tra lại. Đây là chủ ý: thà nói "để shop +kiểm tra" còn hơn bịa ra một con số. + +Hai cách cấp dữ liệu: + +**a) MagiC Knowledge Hub** (khuyến nghị — dùng chung với các workload khác): + +```bash +cd packs +MAGIC_URL=http://localhost:8080 MAGIC_API_KEY=your-key python -m ecomops.seed_knowledge +# hoặc nạp chính sách thật của shop: +python -m ecomops.seed_knowledge /duong/dan/chinh-sach-shop.yaml +``` + +```python +from ecomops import MagicKnowledgeLookup +wf = EcomOpsWorkflow(..., knowledge_lookup=MagicKnowledgeLookup(MAGIC_URL, MAGIC_API_KEY)) +``` + +**b) File YAML tại chỗ** (shop không chạy Knowledge Hub / core in-memory): + +```python +from ecomops import StaticKnowledgeLookup +wf = EcomOpsWorkflow(..., knowledge_lookup=StaticKnowledgeLookup.from_yaml("chinh-sach-shop.yaml")) +``` + +Mỗi entry được gắn 2 tag: `ecomops` + topic (`shipping`, `return`, `order`, +`product`, `complaint`). Lookup lọc theo cả hai, nên câu hỏi về phí ship không bị +trả nhầm chính sách đổi trả, và cũng không kéo về kiến thức khác của tổ chức chỉ +vì trùng từ khoá. + +> ⚠️ `sample_knowledge.yaml` là **dữ liệu mẫu để chạy thử**. Thay bằng chính sách +> thật của shop trước khi dùng với khách — nội dung sai ở đây sẽ thành câu trả lời sai. + +## 5. Demo end-to-end (Zalo + Google Sheet) `example_worker.py` nối: webhook Zalo → tra đơn trong Google Sheet → trả lời có guardrail → gửi lại qua Zalo. @@ -114,7 +153,7 @@ Sheet cần tab `Orders` với dòng header khớp Common Schema (`id`, `customer_id`, `status`, `total_amount`, `shipping_fee`, `created_at`) — xem `connectors/google_sheet/README.md`. -## 5. Giới hạn hiện tại +## 6. Giới hạn hiện tại - Từ khoá phân loại là **danh sách cố định trong code**, chưa cấu hình được qua file. - Chưa có Case Management (tạo case khi khiếu nại/đổi trả) — thuộc Tuần 3–4 của roadmap. @@ -122,3 +161,9 @@ xem `connectors/google_sheet/README.md`. lớn nên thay bằng Nhanh.vn connector hoặc thêm cache. - Guardrail dựa trên cụm từ, không phải mô hình — bắt được các mẫu hứa hẹn phổ biến, nhưng không thay thế được người duyệt với các đơn giá trị cao. + +## 7. Chạy test + +```bash +pytest packs/ecomops/tests +``` diff --git a/packs/ecomops/__init__.py b/packs/ecomops/__init__.py index 224325a..021f177 100644 --- a/packs/ecomops/__init__.py +++ b/packs/ecomops/__init__.py @@ -13,6 +13,7 @@ LLMIntentClassifier, RuleBasedIntentClassifier, ) +from .knowledge import MagicKnowledgeLookup, StaticKnowledgeLookup from .workflow import EcomOpsWorkflow, LLMDrafter, WorkflowResult __all__ = [ @@ -25,6 +26,8 @@ "GuardrailContext", "GuardrailViolation", "Severity", + "MagicKnowledgeLookup", + "StaticKnowledgeLookup", "EcomOpsWorkflow", "LLMDrafter", "WorkflowResult", diff --git a/packs/ecomops/example_worker.py b/packs/ecomops/example_worker.py index bf5263d..9f18895 100644 --- a/packs/ecomops/example_worker.py +++ b/packs/ecomops/example_worker.py @@ -28,6 +28,7 @@ from zalo.webhook import serve # noqa: E402 from ecomops.intents import HybridIntentClassifier, LLMIntentClassifier # noqa: E402 +from ecomops.knowledge import MagicKnowledgeLookup # noqa: E402 from ecomops.workflow import EcomOpsWorkflow, LLMDrafter # noqa: E402 logging.basicConfig(level=logging.INFO) @@ -89,6 +90,10 @@ def main() -> None: classifier=HybridIntentClassifier(llm=LLMIntentClassifier(MAGIC_URL, MAGIC_API_KEY)), drafter=LLMDrafter(MAGIC_URL, MAGIC_API_KEY), order_lookup=make_order_lookup(sheet), + # Chính sách shop lấy từ MagiC Knowledge Hub — nạp trước bằng + # `python -m ecomops.seed_knowledge`, nếu không câu hỏi về phí ship / + # đổi trả sẽ không có dữ liệu để trả lời. + knowledge_lookup=MagicKnowledgeLookup(MAGIC_URL, MAGIC_API_KEY), allowed_contacts=zalo_cfg.get("shop_hotlines", []), ) diff --git a/packs/ecomops/knowledge.py b/packs/ecomops/knowledge.py new file mode 100644 index 0000000..2b55cf8 --- /dev/null +++ b/packs/ecomops/knowledge.py @@ -0,0 +1,163 @@ +"""Knowledge lookup for EcomOps drafts. + +The system prompt tells the model to answer *only* from the "Dữ liệu" block, so +without a knowledge source the shipping-fee and return-policy intents have +nothing to answer from — the bot can only say it will check and get back. This +module supplies that block. + +Two backends: + +- `MagicKnowledgeLookup` — reads MagiC's Knowledge Hub (`GET /api/v1/knowledge`), + so policies live in one place shared with every other MagiC workload. +- `StaticKnowledgeLookup` — a YAML file, for shops running the pack without a + Knowledge Hub (in-memory core, no Postgres). + +Both are plain async callables matching the workflow's `knowledge_lookup` +signature, so they're interchangeable. +""" + +import logging +from pathlib import Path +from typing import Any + +import httpx + +from .intents import Intent + +logger = logging.getLogger("magic_packs.ecomops.knowledge") + +# Tag every EcomOps entry carries, so the pack's lookups don't drag in unrelated +# org knowledge that happens to share a keyword. +ECOMOPS_TAG = "ecomops" + +# Per-intent tag + the keyword query sent to the Hub. The Hub's `q` search is +# keyword-based over title/content, so the query is deliberately in the same +# Vietnamese wording the sample entries use. +INTENT_TOPICS: dict[Intent, tuple[str, str]] = { + Intent.SHIPPING_FEE: ("shipping", "phí ship vận chuyển"), + Intent.RETURN_EXCHANGE: ("return", "đổi trả hoàn tiền bảo hành"), + Intent.ORDER_STATUS: ("order", "trạng thái đơn hàng giao hàng"), + Intent.PRODUCT_INFO: ("product", "sản phẩm size màu chất liệu"), + Intent.COMPLAINT: ("complaint", "khiếu nại xử lý sự cố"), +} + + +def topic_for(intent: Intent) -> str | None: + """The knowledge tag an intent needs, or None if it needs no policy data.""" + topic = INTENT_TOPICS.get(intent) + return topic[0] if topic else None + + +class MagicKnowledgeLookup: + """Pulls shop policy snippets from MagiC's Knowledge Hub.""" + + def __init__(self, magic_url: str, api_key: str = "", top_k: int = 3, timeout: float = 10.0): + self.magic_url = magic_url.rstrip("/") + self.api_key = api_key + self.top_k = top_k + self.timeout = timeout + + async def __call__(self, intent: Intent) -> list[str]: + topic = INTENT_TOPICS.get(intent) + if topic is None: + return [] + tag, query = topic + + headers = {"Authorization": f"Bearer {self.api_key}"} if self.api_key else {} + try: + async with httpx.AsyncClient(timeout=self.timeout) as client: + resp = await client.get( + f"{self.magic_url}/api/v1/knowledge", headers=headers, params={"q": query} + ) + resp.raise_for_status() + entries = resp.json() + except (httpx.HTTPError, ValueError) as e: + # Missing policy data degrades the answer; it shouldn't drop the + # message. The workflow still drafts, and the guardrails still stop + # anything the model invents to fill the gap. + logger.warning("knowledge lookup failed for %s: %s", intent.value, e) + return [] + + if not isinstance(entries, list): + return [] + + # The Hub has no server-side tag filter, so narrow here: an entry must + # carry both the ecomops tag and this intent's topic tag. + snippets = [] + for entry in entries: + if not isinstance(entry, dict): + continue + tags = entry.get("tags") or [] + if ECOMOPS_TAG in tags and tag in tags: + content = (entry.get("content") or "").strip() + if content: + snippets.append(content) + return snippets[: self.top_k] + + +class StaticKnowledgeLookup: + """Serves policy snippets from a local mapping — no Knowledge Hub needed.""" + + def __init__(self, entries: dict[str, list[str]] | None = None): + self.entries = entries or {} + + async def __call__(self, intent: Intent) -> list[str]: + tag = topic_for(intent) + return list(self.entries.get(tag, [])) if tag else [] + + @classmethod + def from_yaml(cls, path: str | Path) -> "StaticKnowledgeLookup": + """Load from a YAML mapping of topic -> list of snippets. + + Imports yaml lazily so the rest of the pack stays dependency-free for + callers that build their knowledge mapping in code. + """ + import yaml + + data = yaml.safe_load(Path(path).read_text(encoding="utf-8")) or {} + return cls({str(k): [str(s) for s in (v or [])] for k, v in data.items()}) + + +def sample_knowledge_path() -> Path: + return Path(__file__).parent / "sample_knowledge.yaml" + + +def knowledge_entries_for_seeding(path: str | Path | None = None) -> list[dict[str, Any]]: + """Turn the sample YAML into `POST /api/v1/knowledge` payloads. + + Each entry is tagged `ecomops` plus its topic so `MagicKnowledgeLookup` can + find it again. + """ + import yaml + + src = Path(path) if path else sample_knowledge_path() + data = yaml.safe_load(src.read_text(encoding="utf-8")) or {} + + payloads = [] + for topic, snippets in data.items(): + for i, snippet in enumerate(snippets or [], start=1): + payloads.append( + { + "title": f"EcomOps — {topic} #{i}", + "content": str(snippet).strip(), + "tags": [ECOMOPS_TAG, str(topic)], + "scope": "org", + } + ) + return payloads + + +async def seed_knowledge_hub( + magic_url: str, api_key: str = "", path: str | Path | None = None, timeout: float = 15.0 +) -> int: + """Push the sample knowledge base into MagiC's Knowledge Hub. Returns count.""" + payloads = knowledge_entries_for_seeding(path) + headers = {"Authorization": f"Bearer {api_key}"} if api_key else {} + + async with httpx.AsyncClient(timeout=timeout) as client: + for payload in payloads: + resp = await client.post( + f"{magic_url.rstrip('/')}/api/v1/knowledge", headers=headers, json=payload + ) + resp.raise_for_status() + return len(payloads) diff --git a/packs/ecomops/requirements.txt b/packs/ecomops/requirements.txt index a51ee1d..10bd7b6 100644 --- a/packs/ecomops/requirements.txt +++ b/packs/ecomops/requirements.txt @@ -1,2 +1,5 @@ # Install the framework first: pip install -e ../../sdk/python[connectors] httpx>=0.27 +# Reading sample_knowledge.yaml / a shop's own policy YAML, and the connector +# configs in example_worker.py. +pyyaml>=6.0 diff --git a/packs/ecomops/sample_knowledge.yaml b/packs/ecomops/sample_knowledge.yaml new file mode 100644 index 0000000..9f9ab19 --- /dev/null +++ b/packs/ecomops/sample_knowledge.yaml @@ -0,0 +1,63 @@ +# Knowledge Base mẫu cho EcomOps. +# +# Đây là DỮ LIỆU MẪU để chạy thử — thay bằng chính sách thật của shop trước khi +# dùng cho khách. Model chỉ được phép trả lời dựa trên nội dung ở đây (xem +# SYSTEM_PROMPT trong prompts.py), nên nội dung sai ở đây sẽ thành câu trả lời sai. +# +# Mỗi khoá là một "topic" khớp với intent (xem INTENT_TOPICS trong knowledge.py): +# shipping | return | order | product | complaint +# +# Nạp vào MagiC Knowledge Hub: +# python -m ecomops.seed_knowledge + +shipping: + - >- + Phí ship nội thành Hà Nội và TP.HCM là 25.000đ, giao trong 1-2 ngày làm việc. + - >- + Phí ship các tỉnh khác là 35.000đ, thời gian giao dự kiến 3-5 ngày làm việc, + vùng sâu vùng xa có thể lâu hơn. + - >- + Miễn phí vận chuyển cho đơn hàng từ 500.000đ trở lên, áp dụng toàn quốc. + +return: + - >- + Shop hỗ trợ đổi trả trong vòng 7 ngày kể từ ngày khách nhận hàng, với điều kiện + sản phẩm còn nguyên tem mác, chưa qua sử dụng và còn đủ hộp/phụ kiện. + - >- + Trường hợp shop giao sai mẫu, sai size hoặc sản phẩm bị lỗi từ nhà sản xuất, + shop chịu toàn bộ phí đổi trả và phí vận chuyển hai chiều. + - >- + Trường hợp khách đổi vì lý do cá nhân (không thích, đổi ý), khách chịu phí + vận chuyển chiều đổi hàng. + - >- + Sản phẩm khuyến mãi trên 50% và đồ lót không áp dụng chính sách đổi trả, + trừ khi bị lỗi từ nhà sản xuất. + +order: + - >- + Các trạng thái đơn hàng: pending (chờ xác nhận), confirmed (đã xác nhận), + shipping (đang giao), delivered (đã giao), cancelled (đã huỷ), + returned (đã hoàn trả). + - >- + Đơn hàng được xác nhận trong vòng 24 giờ kể từ khi đặt. Sau khi chuyển sang + trạng thái shipping, khách có thể tra cứu vị trí đơn qua mã vận đơn. + - >- + Khách có thể huỷ đơn miễn phí khi đơn còn ở trạng thái pending hoặc confirmed. + Đơn đã chuyển sang shipping không huỷ được, khách có thể từ chối nhận hàng. + +product: + - >- + Thông tin size, màu và chất liệu của từng sản phẩm được ghi trong phần mô tả + trên trang sản phẩm. Nếu khách phân vân giữa hai size, shop khuyên chọn size + lớn hơn. + - >- + Tình trạng còn hàng thay đổi liên tục trong ngày. Shop cần kiểm tra kho trước + khi xác nhận cho khách. + +complaint: + - >- + Mọi khiếu nại đều được chuyển cho nhân viên phụ trách xử lý trong vòng + 24 giờ làm việc. Nhân viên sẽ liên hệ lại với khách qua số điện thoại đặt hàng. + - >- + Phương án đền bù (hoàn tiền, đổi hàng, tặng voucher) chỉ do nhân viên quyết định + sau khi kiểm tra đơn — không cam kết trước với khách. diff --git a/packs/ecomops/seed_knowledge.py b/packs/ecomops/seed_knowledge.py new file mode 100644 index 0000000..e9f7006 --- /dev/null +++ b/packs/ecomops/seed_knowledge.py @@ -0,0 +1,40 @@ +"""Nạp Knowledge Base mẫu vào MagiC Knowledge Hub. + + cd packs + MAGIC_URL=http://localhost:8080 MAGIC_API_KEY=your-key python -m ecomops.seed_knowledge + +Truyền đường dẫn file YAML của shop để nạp chính sách thật thay cho dữ liệu mẫu: + + python -m ecomops.seed_knowledge /duong/dan/chinh-sach-shop.yaml +""" + +import asyncio +import logging +import os +import sys + +from .knowledge import knowledge_entries_for_seeding, sample_knowledge_path, seed_knowledge_hub + +logging.basicConfig(level=logging.INFO) +logger = logging.getLogger("ecomops.seed") + + +def main() -> None: + magic_url = os.getenv("MAGIC_URL", "http://localhost:8080") + api_key = os.getenv("MAGIC_API_KEY", "") + path = sys.argv[1] if len(sys.argv) > 1 else sample_knowledge_path() + + entries = knowledge_entries_for_seeding(path) + if path == sample_knowledge_path(): + logger.warning( + "Đang nạp DỮ LIỆU MẪU từ %s — thay bằng chính sách thật của shop trước khi dùng cho khách.", + path, + ) + + logger.info("Nạp %d entry vào %s ...", len(entries), magic_url) + count = asyncio.run(seed_knowledge_hub(magic_url, api_key, path)) + logger.info("Xong: đã nạp %d entry.", count) + + +if __name__ == "__main__": + main() diff --git a/packs/ecomops/tests/test_knowledge.py b/packs/ecomops/tests/test_knowledge.py new file mode 100644 index 0000000..abfe322 --- /dev/null +++ b/packs/ecomops/tests/test_knowledge.py @@ -0,0 +1,164 @@ +"""Knowledge lookup tests — Hub calls mocked via respx, no network.""" + +import httpx +import pytest +import respx + +from ecomops.intents import Intent +from ecomops.knowledge import ( + ECOMOPS_TAG, + MagicKnowledgeLookup, + StaticKnowledgeLookup, + knowledge_entries_for_seeding, + sample_knowledge_path, + seed_knowledge_hub, + topic_for, +) + +MAGIC_URL = "http://magic:8080" +KB_URL = f"{MAGIC_URL}/api/v1/knowledge" + + +def entry(content, tags): + return {"id": "k1", "title": "t", "content": content, "tags": tags} + + +# ---- topic mapping ---- + + +@pytest.mark.parametrize( + "intent,expected", + [ + (Intent.SHIPPING_FEE, "shipping"), + (Intent.RETURN_EXCHANGE, "return"), + (Intent.ORDER_STATUS, "order"), + (Intent.PRODUCT_INFO, "product"), + (Intent.COMPLAINT, "complaint"), + (Intent.GREETING, None), + (Intent.OTHER, None), + ], +) +def test_topic_for(intent, expected): + assert topic_for(intent) == expected + + +# ---- MagicKnowledgeLookup ---- + + +@respx.mock +async def test_returns_matching_entries(): + respx.get(KB_URL).mock( + return_value=httpx.Response(200, json=[entry("Phí ship nội thành 25.000đ", [ECOMOPS_TAG, "shipping"])]) + ) + snippets = await MagicKnowledgeLookup(MAGIC_URL)(Intent.SHIPPING_FEE) + assert snippets == ["Phí ship nội thành 25.000đ"] + + +@respx.mock +async def test_filters_out_other_topics(): + """A keyword hit on the return policy must not answer a shipping question.""" + respx.get(KB_URL).mock( + return_value=httpx.Response( + 200, + json=[ + entry("Đổi trả trong 7 ngày", [ECOMOPS_TAG, "return"]), + entry("Phí ship 25.000đ", [ECOMOPS_TAG, "shipping"]), + ], + ) + ) + assert await MagicKnowledgeLookup(MAGIC_URL)(Intent.SHIPPING_FEE) == ["Phí ship 25.000đ"] + + +@respx.mock +async def test_filters_out_non_ecomops_org_knowledge(): + """Unrelated org knowledge that happens to match the keyword is excluded.""" + respx.get(KB_URL).mock( + return_value=httpx.Response(200, json=[entry("Nội quy công ty về phí ship nội bộ", ["hr", "shipping"])]) + ) + assert await MagicKnowledgeLookup(MAGIC_URL)(Intent.SHIPPING_FEE) == [] + + +@respx.mock +async def test_respects_top_k(): + respx.get(KB_URL).mock( + return_value=httpx.Response(200, json=[entry(f"snippet {i}", [ECOMOPS_TAG, "shipping"]) for i in range(5)]) + ) + assert len(await MagicKnowledgeLookup(MAGIC_URL, top_k=2)(Intent.SHIPPING_FEE)) == 2 + + +async def test_intent_without_a_topic_skips_the_call(): + """Greetings need no policy data — don't spend a request on them.""" + with respx.mock: + route = respx.get(KB_URL).mock(return_value=httpx.Response(200, json=[])) + assert await MagicKnowledgeLookup(MAGIC_URL)(Intent.GREETING) == [] + assert route.call_count == 0 + + +@respx.mock +async def test_hub_error_degrades_to_no_knowledge(): + respx.get(KB_URL).mock(side_effect=httpx.ConnectError("down")) + assert await MagicKnowledgeLookup(MAGIC_URL)(Intent.SHIPPING_FEE) == [] + + +@respx.mock +async def test_unexpected_payload_shape_is_ignored(): + respx.get(KB_URL).mock(return_value=httpx.Response(200, json={"error": "nope"})) + assert await MagicKnowledgeLookup(MAGIC_URL)(Intent.SHIPPING_FEE) == [] + + +@respx.mock +async def test_sends_auth_header_and_query(): + route = respx.get(KB_URL).mock(return_value=httpx.Response(200, json=[])) + await MagicKnowledgeLookup(MAGIC_URL, api_key="secret")(Intent.RETURN_EXCHANGE) + + assert route.calls.last.request.headers["Authorization"] == "Bearer secret" + assert "đổi trả" in route.calls.last.request.url.params["q"] + + +# ---- StaticKnowledgeLookup ---- + + +async def test_static_lookup_by_topic(): + lookup = StaticKnowledgeLookup({"shipping": ["Phí ship 25k"], "return": ["Đổi trả 7 ngày"]}) + assert await lookup(Intent.SHIPPING_FEE) == ["Phí ship 25k"] + assert await lookup(Intent.RETURN_EXCHANGE) == ["Đổi trả 7 ngày"] + assert await lookup(Intent.GREETING) == [] + + +async def test_static_lookup_missing_topic_is_empty(): + assert await StaticKnowledgeLookup({})(Intent.SHIPPING_FEE) == [] + + +async def test_static_lookup_from_sample_yaml(): + lookup = StaticKnowledgeLookup.from_yaml(sample_knowledge_path()) + shipping = await lookup(Intent.SHIPPING_FEE) + assert shipping and any("phí ship" in s.lower() for s in shipping) + + +# ---- seeding ---- + + +def test_sample_yaml_produces_tagged_entries(): + payloads = knowledge_entries_for_seeding() + assert payloads + for p in payloads: + assert ECOMOPS_TAG in p["tags"] + assert len(p["tags"]) == 2 # ecomops + topic + assert p["content"].strip() + assert p["scope"] == "org" + + +def test_every_sample_topic_is_reachable_from_an_intent(): + """A topic in the YAML that no intent maps to would be dead data.""" + topics = {p["tags"][1] for p in knowledge_entries_for_seeding()} + reachable = {topic_for(i) for i in Intent} - {None} + assert topics <= reachable, f"unreachable topics in sample_knowledge.yaml: {topics - reachable}" + + +@respx.mock +async def test_seed_posts_every_entry(): + route = respx.post(KB_URL).mock(return_value=httpx.Response(200, json={"id": "k1"})) + count = await seed_knowledge_hub(MAGIC_URL, api_key="k") + assert count == len(knowledge_entries_for_seeding()) + assert route.call_count == count + assert route.calls.last.request.headers["Authorization"] == "Bearer k" From 33f0d3ad8e441f0896996b6ab81dd12635664444 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 02:27:56 +0000 Subject: [PATCH 2/3] fix(ci): put NOSONAR on the pip line, not the block-scalar key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit My own mistake from the previous commit. Switching the install step to a `run: |` block moved the pip invocation onto a continuation line, but the NOSONAR marker stayed up on the `run: |` key — so it suppressed nothing and SonarCloud re-applied the two supply-chain hotspots (S4830/S5445), dropping the Security Rating to C. Back to a single-line `run:`, matching the neighbouring install/lint steps where this pattern is already proven: the marker is a YAML comment on the same line as the command, stripped before the shell ever sees it. Behaviour is unchanged — Python Tests passed on the previous commit, confirming the find/xargs install and the new pyyaml dependency both work. --- .github/workflows/ci.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0e6a300..8b50f60 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -122,9 +122,10 @@ jobs: # extras — google-auth for the Google Sheet connector, pyyaml for the # EcomOps knowledge base. Every requirements.txt under connectors/ and # packs/ is installed, so adding a new one needs no CI change. - run: | # NOSONAR python:S4830 python:S5445 - find connectors packs -name requirements.txt -print0 \ - | xargs -0 -I{} pip install -r {} + # Kept on one line so the NOSONAR marker lands on the same line as the + # pip invocation — on a `run: |` block it sits on the YAML key instead + # and suppresses nothing. + run: find connectors packs -name requirements.txt -print0 | xargs -0 -I{} pip install -r {} # NOSONAR python:S4830 python:S5445 - name: Lint # Same ephemeral-CI-runner rationale as the installs above. run: pip install ruff && ruff check sdk/python connectors packs # NOSONAR python:S4830 python:S5445 From 0d13139ee6d979c54413b2bb5727c4fd4af6132e Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 02:31:16 +0000 Subject: [PATCH 3/3] fix(security): validate knowledge file paths before reading them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SonarCloud flagged a high-severity path-injection vulnerability in the code I added in this PR — and it was right, so this fixes it rather than suppressing it. `seed_knowledge.py` passes sys.argv[1] straight into `knowledge_entries_for_seeding()`, which did `Path(path).read_text()` with no validation, and everything it reads is POSTed into the Knowledge Hub. That turns "seed the knowledge base" into "copy any file this process can read into a searchable store" — an arbitrary-file read whose output lands somewhere it can be queried back out. `StaticKnowledgeLookup.from_yaml` had the same hole. Both now go through `_resolve_yaml_path()`, which requires an existing regular file with a .yaml/.yml suffix. It calls `resolve()` first, deliberately: that collapses `..` and follows symlinks, so a `policy.yaml` symlinked at a private key is judged by its real name and fails the suffix check instead of sailing through it. The seeder CLI now exits 2 with a clear message instead of propagating a traceback. Verified by hand that /etc/hostname, a suffix-less file and a .yaml symlink to a key file are all refused, and that a genuine policy YAML still loads. Note on the previous commit: it moved a NOSONAR marker in ci.yml on the theory that the block-scalar placement was what dropped the Security Rating. That placement was genuinely wrong and is worth keeping fixed, but it was not the cause — this was. The rule key only became visible once the finding surfaced as a code-scanning review comment on knowledge.py:134. Tests: 7 new (non-YAML suffix, missing file, directory, symlink disguise, traversal, valid file, and the same validation on StaticKnowledgeLookup). 198 total, all passing. --- packs/ecomops/knowledge.py | 30 +++++++++++++- packs/ecomops/seed_knowledge.py | 7 +++- packs/ecomops/tests/test_knowledge.py | 59 +++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 3 deletions(-) diff --git a/packs/ecomops/knowledge.py b/packs/ecomops/knowledge.py index 2b55cf8..9773c2e 100644 --- a/packs/ecomops/knowledge.py +++ b/packs/ecomops/knowledge.py @@ -114,7 +114,7 @@ def from_yaml(cls, path: str | Path) -> "StaticKnowledgeLookup": """ import yaml - data = yaml.safe_load(Path(path).read_text(encoding="utf-8")) or {} + data = yaml.safe_load(_resolve_yaml_path(path).read_text(encoding="utf-8")) or {} return cls({str(k): [str(s) for s in (v or [])] for k, v in data.items()}) @@ -122,6 +122,32 @@ def sample_knowledge_path() -> Path: return Path(__file__).parent / "sample_knowledge.yaml" +_ALLOWED_SUFFIXES = {".yaml", ".yml"} + + +def _resolve_yaml_path(path: str | Path) -> Path: + """Validate a caller-supplied knowledge path before touching the filesystem. + + `seed_knowledge.py` takes this straight from argv and everything it reads is + POSTed into the Knowledge Hub, where it becomes queryable. An unvalidated + path therefore turns "seed the knowledge base" into "copy any file this + process can read into a searchable store" — so constrain it to an existing + regular YAML file. + + `resolve()` runs first on purpose: it collapses `..` and follows symlinks, so + a `policy.yaml` symlinked at `/etc/shadow` is checked by its real name and + fails the suffix test rather than sailing through it. + """ + resolved = Path(path).expanduser().resolve() + if resolved.suffix.lower() not in _ALLOWED_SUFFIXES: + raise ValueError( + f"knowledge file must be .yaml or .yml, got {resolved.suffix or '(no suffix)'}: {resolved}" + ) + if not resolved.is_file(): + raise FileNotFoundError(f"knowledge file not found: {resolved}") + return resolved + + def knowledge_entries_for_seeding(path: str | Path | None = None) -> list[dict[str, Any]]: """Turn the sample YAML into `POST /api/v1/knowledge` payloads. @@ -130,7 +156,7 @@ def knowledge_entries_for_seeding(path: str | Path | None = None) -> list[dict[s """ import yaml - src = Path(path) if path else sample_knowledge_path() + src = _resolve_yaml_path(path) if path else sample_knowledge_path() data = yaml.safe_load(src.read_text(encoding="utf-8")) or {} payloads = [] diff --git a/packs/ecomops/seed_knowledge.py b/packs/ecomops/seed_knowledge.py index e9f7006..f12b687 100644 --- a/packs/ecomops/seed_knowledge.py +++ b/packs/ecomops/seed_knowledge.py @@ -24,7 +24,12 @@ def main() -> None: api_key = os.getenv("MAGIC_API_KEY", "") path = sys.argv[1] if len(sys.argv) > 1 else sample_knowledge_path() - entries = knowledge_entries_for_seeding(path) + try: + entries = knowledge_entries_for_seeding(path) + except (ValueError, FileNotFoundError) as e: + logger.error("%s", e) + sys.exit(2) + if path == sample_knowledge_path(): logger.warning( "Đang nạp DỮ LIỆU MẪU từ %s — thay bằng chính sách thật của shop trước khi dùng cho khách.", diff --git a/packs/ecomops/tests/test_knowledge.py b/packs/ecomops/tests/test_knowledge.py index abfe322..6902fe3 100644 --- a/packs/ecomops/tests/test_knowledge.py +++ b/packs/ecomops/tests/test_knowledge.py @@ -155,6 +155,65 @@ def test_every_sample_topic_is_reachable_from_an_intent(): assert topics <= reachable, f"unreachable topics in sample_knowledge.yaml: {topics - reachable}" +# ---- path validation ---- +# +# seed_knowledge.py passes argv straight through, and everything read here ends +# up POSTed into the Knowledge Hub — so an unvalidated path is an arbitrary-file +# read that lands in a queryable store. + + +def test_rejects_non_yaml_suffix(tmp_path): + secret = tmp_path / "passwd" + secret.write_text("root:x:0:0") + with pytest.raises(ValueError, match="must be .yaml"): + knowledge_entries_for_seeding(secret) + + +def test_rejects_missing_file(tmp_path): + with pytest.raises(FileNotFoundError): + knowledge_entries_for_seeding(tmp_path / "khong-ton-tai.yaml") + + +def test_rejects_directory(tmp_path): + d = tmp_path / "policies.yaml" + d.mkdir() + with pytest.raises(FileNotFoundError): + knowledge_entries_for_seeding(d) + + +def test_symlink_cannot_disguise_a_non_yaml_target(tmp_path): + """resolve() runs before the suffix check, so a .yaml symlink pointing at a + non-YAML file is judged by its real name.""" + secret = tmp_path / "id_rsa" + secret.write_text("PRIVATE KEY") + link = tmp_path / "policy.yaml" + link.symlink_to(secret) + + with pytest.raises(ValueError, match="must be .yaml"): + knowledge_entries_for_seeding(link) + + +def test_traversal_is_normalized_and_still_checked(tmp_path): + with pytest.raises((ValueError, FileNotFoundError)): + knowledge_entries_for_seeding(tmp_path / ".." / ".." / "etc" / "passwd") + + +def test_accepts_a_real_yaml_file(tmp_path): + policy = tmp_path / "chinh-sach.yaml" + policy.write_text("shipping:\n - Phí ship 30k\n", encoding="utf-8") + + payloads = knowledge_entries_for_seeding(policy) + + assert [p["content"] for p in payloads] == ["Phí ship 30k"] + + +def test_static_lookup_applies_the_same_validation(tmp_path): + secret = tmp_path / "secrets.env" + secret.write_text("TOKEN=abc") + with pytest.raises(ValueError, match="must be .yaml"): + StaticKnowledgeLookup.from_yaml(secret) + + @respx.mock async def test_seed_posts_every_entry(): route = respx.post(KB_URL).mock(return_value=httpx.Response(200, json={"id": "k1"}))