fix(benchmarks): avoid logging sensitive response bodies by loks0n · Pull Request #14679 · appwrite/appwrite · GitHub
Skip to content

fix(benchmarks): avoid logging sensitive response bodies - #14679

Open
loks0n wants to merge 1 commit into
mainfrom
codex/triage-code-scanning
Open

loks0n wants to merge 1 commit into
mainfrom
codex/triage-code-scanning

Conversation

@loks0n

@loks0n loks0n commented Oct 19, 2026

Copy link
Copy Markdown
Member

Failed benchmark setup calls currently print the complete HTTP response body, which can expose API keys or session data in logs (CodeQL #511). Remove that body log and retain the assertion name, HTTP status, and thrown error.

Validation: exercised the actual assertion function with a failing response containing a sentinel secret. The original code logs it; the changed code keeps the status and failure while omitting the body. JavaScript syntax check and git diff --check pass.

Related triage: #14675. The neighboring status-only alert #510 is a false positive and is dismissed separately. Full benchmark E2E was not run; this changes only failure logging.

@loks0n loks0n self-assigned this Oct 19, 2026
@tenki-reviewer

tenki-reviewer Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

@hansi-codes

hansi-codes Bot commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

🟢 Tier S · Ready to merge

The bulk-operations benchmark assertion helper no longer logs HTTP response bodies when an assertion fails. It continues to report the assertion name and response status, then throws the assertion error.

Verdict New comments Fixed Still open
✅ Approved 0 0 0
📂 Walkthrough · 1
File Change
tests/benchmarks/bulk-operations/utils.js Removed the response-body log from failed assertions while retaining the status and thrown error.

Reviewed f576d49 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Tier S · Looks good to merge. Summary

@github-actions

Copy link
Copy Markdown

Security rules

No new WARNING or ERROR findings from security rules.

19 existing findings tracked in .semgrep/baseline.json
  • php.appwrite.guest-write-without-abuse-limit (12)
  • php.appwrite.secret-compare-timing (5)
  • php.appwrite.weak-secret-env-default (2)

Posted by Checks / Rules. Re-runs update this comment in place. Rule details and baseline: .semgrep/README.md.

@github-actions

Copy link
Copy Markdown

✨ Benchmark results

Comparing main (before) → codex/triage-code-scanning (after).

Metric Before After Change
🚀 Requests/sec 200.6 196.34 ⚪ -2.1%
⏱️ Latency P50 85.45 ms 89.97 ms 🔴 +5.3%
⏱️ Latency P95 197.29 ms 200.87 ms ⚪ +1.8%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 89.97 200.87 12,540 196.34 +3.59
Account 161.76 293.3 660 10.98 +2.3
TablesDB 88.07 164.62 6,820 108.81 +8.84
Storage 80.74 188.4 3,300 54.53 -2.62
Functions 120.77 220.9 1,760 29.86 +6.92

Top API waits (after)

API request Max wait (ms)
storage.buckets.create 484.46
account.name.update 437.14
storage.files.create 383.86
account.prefs.update 378.93
tablesdb.rows.update 339.47

This branch has not been deployed

No deployments
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.

1 participant