Skip to content

fix(ci): allowlist Ascend diagnostic artifacts - #148

Merged
ShuhaoZhangTony merged 6 commits into
mainfrom
feature/security-redact-diagnostics
Jul 20, 2026
Merged

fix(ci): allowlist Ascend diagnostic artifacts#148
ShuhaoZhangTony merged 6 commits into
mainfrom
feature/security-redact-diagnostics

Conversation

@ShuhaoZhangTony

@ShuhaoZhangTony ShuhaoZhangTony commented Jul 19, 2026

Copy link
Copy Markdown
Member

Summary

  • replace the raw Ascend benchmark environment artifact with deterministic JSON generated from an exact key allowlist
  • route all environment-derived diagnostic fields through the same deny-by-default collector
  • add dynamic regression tests that exclude synthetic GitHub, HF, publication, SSH, proxy, and future VLLM_* credential variables by both key and value

Root cause

collect_ascend_diagnostics wrote env | sort to the benchmark result tree. The workflow uploads that tree with if: always(), while GitHub log masking does not sanitize artifact file contents.

Validation

Implementation head: 9fabfa56572b74dd14abc852942296c1bb4f0d0a
Current head: ac878c106bcfe23807ac184ba69db0ac80316306

  • focused CI suites: 40 passed
  • Ruff: passed
  • Python compilation: passed
  • bash -n: passed
  • raw environment dump scan: passed
  • synthetic clean-environment redaction test: passed
  • git diff --check: passed

The current head includes the tested Ascend smoke isolation from #155; this PR remains stacked behind #155 and must pass its own exact-head smoke before merge.

@ShuhaoZhangTony
ShuhaoZhangTony marked this pull request as ready for review July 19, 2026 17:15
Replace the raw environment artifact with a deterministic exact-key allowlist and cover credential-shaped variables with synthetic redaction tests.

Signed-off-by: Shuhao Zhang (Tony) <shuhao_zhang@hust.edu.cn>
@ShuhaoZhangTony
ShuhaoZhangTony force-pushed the feature/security-redact-diagnostics branch from a0b1d12 to 9fabfa5 Compare July 19, 2026 17:38
@ShuhaoZhangTony

Copy link
Copy Markdown
Member Author

@moonandlife @iliujunn @QixinZhang26 @Renty-0 — independent review is requested for this security hardening. It replaces raw diagnostic environment capture with an exact allowlist and synthetic credential-exclusion regressions. The affected benchmark workflow remains disabled until this PR receives the required approval and merges.

@ShuhaoZhangTony ShuhaoZhangTony added priority: high High-priority correctness or user impact ready labels Jul 19, 2026
ShuhaoZhangTony and others added 4 commits July 20, 2026 10:57
Signed-off-by: Shuhao Zhang (Tony) <5267690+ShuhaoZhangTony@users.noreply.github.com>
Signed-off-by: Shuhao Zhang (Tony) <5267690+ShuhaoZhangTony@users.noreply.github.com>

# Conflicts:
#	.github/workflows/scripts/run_e2e_inference_regression.sh
#	.github/workflows/scripts/run_e2e_serve_smoke.sh
#	tests/ci/test_ascend_benchmark_script_static.py
Signed-off-by: Shuhao Zhang (Tony) <5267690+ShuhaoZhangTony@users.noreply.github.com>
Signed-off-by: Shuhao Zhang (Tony) <shuhao_zhang@hust.edu.cn>
Signed-off-by: Shuhao Zhang (Tony) <shuhao_zhang@hust.edu.cn>
@ShuhaoZhangTony

Copy link
Copy Markdown
Member Author

Local maintainer validation completed at exact head ac878c106bcfe23807ac184ba69db0ac80316306: focused diagnostic allowlist/workflow suites 25 passed; Ruff, Bash syntax, and diff-check passed. Qwen2.5-7B exact-head Ascend serve smoke passed on NPU 2, and device/port cleanup was verified.

@ShuhaoZhangTony
ShuhaoZhangTony merged commit 614fa13 into main Jul 20, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: high High-priority correctness or user impact ready

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant