Skip to content

docs(bytecode): document trusted input boundary#746

Merged
buke merged 2 commits into
mainfrom
docs/bytecode-trust-boundary
May 22, 2026
Merged

docs(bytecode): document trusted input boundary#746
buke merged 2 commits into
mainfrom
docs/bytecode-trust-boundary

Conversation

@buke
Copy link
Copy Markdown
Owner

@buke buke commented May 22, 2026

  • warn that QuickJS bytecode must only be loaded from trusted sources
  • annotate EvalBytecode and LoadModuleBytecode with the same trust requirement
  • mirror the guidance in both the English and Chinese README bytecode sections

- warn that QuickJS bytecode must only be loaded from trusted sources
- annotate EvalBytecode and LoadModuleBytecode with the same trust requirement
- mirror the guidance in both the English and Chinese README bytecode sections
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds security warnings to the README and relevant function docstrings in context.go, cautioning against loading QuickJS bytecode from untrusted sources due to memory corruption risks. Review feedback suggests further refining these docstrings to correct typos, remove inaccurate parameter descriptions, and ensure the security warnings are explicit and consistent across the codebase.

Comment thread context.go Outdated
Comment thread context.go Outdated
@codecov
Copy link
Copy Markdown

codecov Bot commented May 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (f04fc26) to head (cf21d54).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #746   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           17        17           
  Lines         3705      3705           
=========================================
  Hits          3705      3705           
Files with missing lines Coverage Δ
context.go 100.00% <ø> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f04fc26...cf21d54. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

- fix the LoadModuleBytecode docstring name and parameter wording
- make the memory corruption risk explicit for bytecode-loading APIs
- tighten the EvalBytecode ownership note to match the returned value
@buke
Copy link
Copy Markdown
Owner Author

buke commented May 22, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request adds security warnings to the English and Chinese README files and the context.go source file regarding the loading of QuickJS bytecode. The documentation now emphasizes that bytecode should only be loaded from trusted sources to prevent potential memory corruption. A review comment suggests further improving the documentation for LoadModuleBytecode by explicitly stating that the caller is responsible for calling Free() on the returned Value, ensuring consistency with the updated EvalBytecode method.

Comment thread context.go
@buke buke merged commit ec1afe7 into main May 22, 2026
8 checks passed
@buke buke deleted the docs/bytecode-trust-boundary branch May 22, 2026 07:29
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