Merged
Conversation
…DR plugin
The provider test-credentials flow invokes
``ToolRegistry.execute(tool_name, **params)`` with a freshly-created
``ToolContext`` that has no ``params`` attribute. The script-handler wrapper
in ``tool_loader._build_script_handler`` was forwarding every kwarg to the
target function unconditionally, which broke legacy plugin handlers (like
sangfor_xdr) whose entry points are declared as ``async def run(ctx)`` and
read parameters from ``ctx.params``. Symptoms observed in production:
* ``run_alerts() got an unexpected keyword argument 'action'``
* ``'ToolContext' object has no attribute 'params'``
* ``'utf-8' codec can't decode byte 0x8d in position 0`` on every probe
Changes:
* ``flocks/tool/tool_loader.py`` — inspect the wrapped function's signature
once, inject ``ctx.params = kwargs`` so legacy handlers keep working, and
filter the forwarded kwargs to those the function actually accepts (or
pass everything when ``**kwargs`` is declared).
* ``.flocks/plugins/tools/api/sangfor_xdr/sangfor_xdr.handler.py``:
- normalise the configured ``host`` (strip ``http(s)://``, trailing
slash, optional inline ``:port``) so a WebUI value of
``https://10.0.0.1`` no longer becomes ``https://https://10.0.0.1``;
- reject non-hex / empty ``auth_code`` with an actionable message
instead of a cryptic ``binascii.Error``;
- tolerate non-UTF-8 bytes in the AES-CBC plaintext;
- replace the brittle ``resp.json()`` call with ``_parse_response_body``
that decodes raw bytes through ``utf-8 / utf-8-sig / gbk / gb18030 /
latin-1`` and surfaces a deterministic ``RuntimeError`` instead of
leaking ``UnicodeDecodeError``;
- request ``Accept-Encoding: identity`` to avoid appliance-side gzip
that aiohttp cannot transparently inflate on every code path.
Tests:
* ``tests/tool/test_script_handler_wrapper.py`` — covers the three
handler signature shapes plus ``ctx.params`` injection.
* ``tests/tool/test_sangfor_xdr_handler.py`` — parametrised host
normalisation, auth_code validation, and response decoding fallbacks
(including the exact 0x8D leading-byte failure mode).
Made-with: Cursor
The handler's `_aes_cbc_decrypt` historically called `cipher.encryptor()` instead of `cipher.decryptor()`. This silently produced a re-encrypted blob in place of the AK/SK, so every signed request was rejected by XDR with `access key not exist` / `Full ak/sk authentication is required`, and the previous "tolerate non-UTF-8 plaintext" workaround merely masked the symptom. Aligns the implementation with the official Sangfor demo (`aksk_py3.Signature.__aes_cbc_decrypt`): * AES-CBC decryption with a zero IV, NUL-stripped, strict UTF-8 decode. * Sort query string keys in `_sign_request` to match `__query_str_transform`, so multi-param GETs sign deterministically. Adds regression tests: * `test_aes_cbc_decrypt_round_trips_against_reference_encryption` encrypts a known plaintext and asserts the handler decrypts it back, preventing reintroduction of `encryptor()`. * `test_sign_request_sorts_query_params` pins the canonical-query ordering so two dicts with the same items in different orders sign identically. Made-with: Cursor
Audited every `_run_request(...)` call against the official 接口开放列表
(Sangfor XDR open API HTML export, 113 endpoints) and corrected six
deviations that produced 404 / signature mismatch / 405 against real
appliances:
* alerts:
- `get_proof` POST → GET (spec: GET /alerts/:uuid/proof)
- `get_detail` removed (no such endpoint in 开放列表;
detail must be derived from list+uuId)
* incidents:
- `get_proof` POST → GET (spec: GET /incidents/:uuid/proof)
- `get_entities` POST → GET (spec: GET /incidents/:uuid/entities/
{dns,file,host,innerip,ip,process})
- `get_detail` removed (no such endpoint in 开放列表)
* vulns:
- `vuln_list` /vuls/list → /vuls/risk/list
(legacy path missing from 开放列表;
spec: POST /vuls/risk/list 获取漏洞、
弱密码数据)
YAML schemas (alerts/incidents/vulns) updated to drop removed actions
from the enum, fix descriptions, and call out that detail views should
go through `list` filtering — keeping the prompt surface aligned with
what the appliance actually accepts.
Made-with: Cursor
Self-review of the previous three commits surfaced two latent issues
that would only fire on edge inputs but silently break signing /
routing. Both are now covered by tests.
* sangfor_xdr handler — replace the hand-rolled ``host`` normalisation
with ``urllib.parse.urlparse`` so any URL form collapses to a clean
``scheme://host[:port]`` base and never leaks the path/query/fragment
into ``base_url``. Previously:
- ``https://10.0.0.1/api`` produced
``https://10.0.0.1/api/api/xdr/v1/...`` (404 + signature drift)
- ``https://10.0.0.1?x=1`` / ``#frag`` leaked the suffix
- ``https://[::1]:8443`` lost the IPv6 brackets
All three are now exercised by ``test_resolve_runtime_config_
normalises_host`` (parametrised, +6 cases).
* tool_loader ``_build_script_handler`` — drop the first positional
parameter (always our injected ``ctx``) from ``fn_param_names`` so a
user-supplied kwarg literally named ``ctx`` cannot trigger
``TypeError: got multiple values for argument 'ctx'`` when forwarded
to the wrapped function.
No behaviour change for any handler signature already in tree; the
existing 4 wrapper tests + 23 sangfor_xdr tests still pass.
Made-with: Cursor
XDR API connectivity test now passes the auth handshake but seven POST
endpoints still failed with "参数解析异常 / 请求参数校验失败 /
参数不合法":
* whitelists/list, assets/list, assets/ipsegmenttree,
vuls/baseline/list, vuls/risk/list, responses/host/isolate/list — all
send body=`{}`.
* alerts/dealstatus/list, incidents/dealstatus/list — per the 开放接口列表
spec these expect a JSON *array* body (`["alert-uuid", ...]`), not a
dict.
Root cause: `_request` used `payload = json.dumps(data) if data else ""`,
which (a) treated empty containers as falsy and silently transmitted an
empty string body that the appliance refuses to parse, and (b) caused
the signed payload hash to be SHA256("") instead of SHA256("{}"), which
also breaks signature verification on versions that *do* accept empty
JSON.
Fixes:
* `_request` now serialises any non-None `data` (including `{}` / `[]`)
via `json.dumps(..., ensure_ascii=False)` and only sends an empty body
when `data is None`. Type annotation widened from
`Optional[dict[str, Any]]` to `Optional[Any]` so list bodies are
first-class.
* `run_alerts(action="status_list")` and
`run_incidents(action="status_list")` now build a JSON array body from
the optional `uuids` parameter (string CSV is also accepted) instead
of `{}`. YAML schemas updated to document the new semantics.
* Added 6 parametrised regression tests covering wire-level body
serialisation for `{}`, `[]`, populated dict/list and `None`, plus a
GET assertion that the body kwarg is omitted entirely.
Made-with: Cursor
Second WebUI connectivity report narrowed the failure surface to two
remaining endpoints:
* whitelists.list → "param page cannot be null"
Spec (开放接口列表 → POST /whitelists/list) names the paging key
``page``; we were sending ``pageNum`` so the appliance's strict
validator never found it.
* vulns.vuln_list → "请求参数校验失败"
Spec marks ``dataType`` as the only paramNotNull=0 (required) field
on POST /vuls/risk/list. Omitting it triggers param validation
failure regardless of paging.
Changes:
* run_whitelists("list") now sends ``{"page", "pageSize"}`` with safe
defaults (page=1, pageSize=20) so the WebUI probe — which passes no
params — still satisfies the validator. Legacy ``page_num`` callers
continue to work via aliasing.
* run_vulns("vuln_list") now defaults to ``dataType="loophole"`` and
exposes a ``data_type`` enum (loophole|weakpwd) in the YAML schema.
Paging keys also normalised to ``page``/``pageSize``.
* run_vulns("baseline") normalised to ``page``/``pageSize`` for spec
parity (was already passing because previous paging keys were
optional on this endpoint).
* 5 new regression tests assert the wire-level body shape so any future
refactor that re-introduces ``pageNum`` or drops ``dataType`` will
fail loudly instead of only surfacing on a live appliance.
Made-with: Cursor
The provider's ``_provider.yaml`` declared ``verify_ssl`` as a free-text credential field, but ServiceDetailPanel already renders a generic "SSL 验证" toggle for every API service. Both controls write to the same ``raw_config["verify_ssl"]`` key, so the WebUI showed two SSL inputs side-by-side (a "Verify SSL" text box that accepted "true"/"false" strings, and the toggle directly below it). Removing the credential-field declaration leaves the canonical toggle as the single source of truth. Handler behaviour is unchanged because ``_resolve_runtime_config`` already reads ``raw["verify_ssl"]`` regardless of where it was set. Notes section also updated to direct users to the SSL toggle. Made-with: Cursor
xiami762
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix(sangfor-xdr): drop duplicate Verify SSL field from credential form
The provider's
_provider.yamldeclaredverify_sslas a free-textcredential field, but ServiceDetailPanel already renders a generic
"SSL 验证" toggle for every API service. Both controls write to the
same
raw_config["verify_ssl"]key, so the WebUI showed two SSLinputs side-by-side (a "Verify SSL" text box that accepted "true"/"false"
strings, and the toggle directly below it).
Removing the credential-field declaration leaves the canonical toggle as
the single source of truth. Handler behaviour is unchanged because
_resolve_runtime_configalready readsraw["verify_ssl"]regardlessof where it was set.
Notes section also updated to direct users to the SSL toggle.