7.7 KiB
OAOPS1 Patrol Review Findings
Review Scope
Reviewed:
scripts/qa/run-oa-real-use-daily.shscripts/qa/run-oa-final-launch-readiness.shscripts/qa/check-oa-only-smoke-readiness.mjsdocs/管理员手册.mddocs/文档事实源状态矩阵.md
Commands run:
bash -n scripts/qa/run-oa-real-use-daily.shbash -n scripts/qa/run-oa-final-launch-readiness.sh
No deployment, public OA writes, Enterprise WeChat writes, payment actions or database restore were performed.
Findings
High: Daily patrol can hide child PASS_WITH_FINDINGS results
run-oa-real-use-daily.sh wraps child checks with run_finding_step, which marks a step as PASS whenever the command exits 0. check-oa-only-smoke-readiness.mjs returns exit 0 when role credentials are missing but emits summary status PASS_WITH_FINDINGS. The parent summary only inspects parent step status plus the local authenticated probe, so child PASS_WITH_FINDINGS can be lost and the daily patrol can conclude PASS_OA_REAL_USE_DAILY_READY.
Why it matters:
docs/管理员手册.mdrequires missing live smoke credentials to be recorded as finding or SKIP, not disguised as pass.- OALAUNCH1 reads
latestOaops.findingCount; if the daily report loses child findings, final readiness can understate risk.
Evidence:
scripts/qa/run-oa-real-use-daily.shlines 63-66 mark a child command asPASSonly by exit code.scripts/qa/check-oa-only-smoke-readiness.mjslines 253-259 emitsPASS_WITH_FINDINGSwhen credentials are skipped.scripts/qa/check-oa-only-smoke-readiness.mjsline 288 exits non-zero only for blocking failures.scripts/qa/run-oa-real-use-daily.shlines 412-426 build findings without reading the child summary statuses.
Recommended fix:
- Add a report-aware step wrapper for child reports. After each child command, read its
summary.json; promotePASS_WITH_FINDINGS,SKIP,PARTIAL,PAUSEDand childfindingCount > 0into parent findings. - Include
oa-only-smoke-readinessinchildReports, not justlive-readiness.
High: Privacy smoke failure can leave summary.json saying PASS
run-oa-real-use-daily.sh writes summary.json before running report-privacy-smoke. If privacy smoke fails, it sets STATUS=1 and exits non-zero, but the already-written summary is not regenerated. The report can therefore contain a PASS conclusion while the script fails.
Why it matters:
- Day 7 review is driven by
summary.json. - A privacy violation must be visible in the machine-readable report, not only in process exit code or console output.
Evidence:
scripts/qa/run-oa-real-use-daily.shlines 505-510 write summary, then run privacy smoke, then exit.scripts/qa/run-oa-final-launch-readiness.shlines 483-486 already has the better pattern: rewrite summary when privacy status changes.
Recommended fix:
- Mirror the OALAUNCH1 pattern in OAOPS1: run privacy smoke after summary, then rewrite summary if status changed.
- Add an explicit
report-privacy-smokestep/finding to the daily summary.
Medium: Privacy smoke prints raw matches to stdout
OAOPS1 privacy smoke prints the full grep matches when it detects sensitive patterns. If the match is a token, phone number, email or raw key line, the scanner itself leaks it into terminal/CI logs.
Why it matters:
- Privacy tooling should not echo the sensitive value it detected.
- This violates the current report discipline: status/count/path summaries only.
Evidence:
scripts/qa/run-oa-real-use-daily.shlines 347-351 capture and print raw grep matches.
Recommended fix:
- Store only redacted match metadata: file path, line number and rule name.
- Do not print full matched lines.
Medium: Daily summary writes full local paths
OAOPS1 step JSON and summary use absolute local paths for logPath and reportDir. Current project privacy rules prohibit complete local paths in task/docs/reports.
Why it matters:
- The final launch script already uses repo-relative
reportReffor this reason. - These daily reports may be copied into Trellis task evidence or user-facing summaries.
Evidence:
scripts/qa/run-oa-real-use-daily.shline 35 writes absolutelogPath.scripts/qa/run-oa-real-use-daily.shline 380 writes absolute missing-steplogPath.scripts/qa/run-oa-real-use-daily.shline 443 writes absolutereportDir.scripts/qa/run-oa-final-launch-readiness.shlines 160-167 use safer relativereportRef.
Recommended fix:
- Add a
reportRef(file)helper to OAOPS1 and use repo-relative paths when possible. - Replace
reportDirwithreportRefor keepreportDirNameonly.
Medium: Read-only probe can record business totals, not only counts/status
The authenticated read-only probe records any short string field that is not excluded by a narrow key regex. In observed evidence, reimbursement aggregate amount appears in the patrol report. The report policy says generated evidence should keep status/count/report-path summaries, not business amounts.
Why it matters:
- Aggregate amounts can still be sensitive business information in a small team.
- OALAUNCH evidence should remain safe to copy into task notes.
Evidence:
scripts/qa/run-oa-real-use-daily.shlines 197-207 allow numeric/boolean values and short strings.- Observed latest daily report includes a reimbursement
totalAmountaggregate in the authenticated probe summary.
Recommended fix:
- Change
summarizePayloadto an allowlist: item counts, totals as record counts only, status/mode/conclusion and booleans that are operational states. - Drop amount, money, balance, price, cost, bank and invoice-value-like fields.
Overall Assessment
The patrol function is directionally useful and already covers the right operational domains: public readiness, live smoke, feature flags, OA-only roles, read-only business probes, mobile/WeCom entry and privacy checks.
Before starting the formal 7-day OAOPS1 window, it should be improved in a small technical task. The most important fix is report-status correctness: parent reports must faithfully include child findings and privacy failures.
Suggested Improvement Order
- Fix child summary propagation and parent
findingCount. - Fix privacy-smoke summary rewrite and redacted match output.
- Replace absolute paths with report refs.
- Reduce authenticated probe payload summaries to operational counts/status only.
- Re-run
bash -n, default OAOPS1 daily, and default OALAUNCH1 readiness.
Implementation Result
Implemented in scripts/qa/run-oa-real-use-daily.sh:
- Step JSON now writes repo-relative
logRefinstead of absolutelogPath. - Daily summary now writes
reportRefinstead of absolutereportDir. - Parent daily summary reads child
summary.jsonforlive-smoke-readinessandoa-only-smoke-readiness. - Child
PASS_WITH_FINDINGS,SKIP,MISSING,PAUSED,PARTIAL, blocker statuses and child finding counts now contribute to parent findings. - Privacy smoke now writes redacted
privacy-findings.txtmetadata instead of printing raw grep matches. - Privacy smoke now records a
report-privacy-smokestep and summary is rewritten after privacy checks. - Authenticated probe payload summary now uses an allowlist and excludes amount, money, price, cost, balance, bank, receipt, account, invoice, tax, fee, paid and payment fields.
Final validation report:
.trellis/tasks/05-26-oaops1-patrol-feature-review/research/oaops1-after-patrol-fix-final/summary.json- Status:
PASS_WITH_FINDINGS - Parent finding count: 4
childReports.oaOnlySmokeReadiness.status:PASS_WITH_FINDINGSchildReports.oaOnlySmokeReadiness.findingCount: 7report-privacy-smoke:PASS- No full local path matches in
summary.json. - No business amount key matches in
summary.jsonorauthenticated-readonly-probe.json.