Skip to content

Bind reward JavaScript placeholders as data - #302

Closed
BenCodez wants to merge 63 commits into
masterfrom
security/javascript-placeholder-bindings
Closed

Bind reward JavaScript placeholders as data#302
BenCodez wants to merge 63 commits into
masterfrom
security/javascript-placeholder-bindings

Conversation

@BenCodez

@BenCodez BenCodez commented Aug 26, 2026

Copy link
Copy Markdown
Owner

Summary

  • replace the superseded lexer-heavy Harden reward JavaScript placeholder evaluation #293 approach with centralized, automatic placeholder handling in JavascriptEngine
  • keep existing JavaScript unchanged: normal expressions such as Player.hasPermission("someper") == true continue to run as-is
  • automatically resolve PlaceholderAPI %placeholder% values and AdvancedCore %name% / {name} custom placeholders anywhere JavaScript is evaluated
  • preserve existing placeholder usage in expressions, quoted strings, template text/template expressions, and regular expressions without requiring config migration
  • pass expression placeholder values to the script engine as bound data instead of copying untrusted placeholder output into executable source
  • preserve string semantics for placeholders already inside quoted/template/regex source and escape resolved values for that literal context
  • prevent placeholder output from manufacturing a new executable [Javascript=...] marker in the same processing path
  • package the Nashorn parser API used for source-context classification and keep it available even if the active JavaScript engine is Rhino/GraalJS

Automatic compatibility

No JavaScript configuration changes should be required. Placeholder handling happens inside JavascriptEngine, so callers do not need to remember a separate preprocessing step.

Examples that remain valid:

Player.hasPermission("someper") == true
%permission_result% == true
'%player_name%' == 'Ben'
'Hello %player_name%!'
`Hello %player_name%`
/^%player_name%$/.test(value)
{count} > 0

Security boundary

Operator-authored JavaScript remains executable code. PlaceholderAPI/reward/custom placeholder output is treated as data and cannot change the JavaScript program structure. Expression values are engine-bound; values inside existing string/template/regex literals are escaped for that literal context.

This supersedes #293 without carrying forward its handwritten JavaScript lexer.

@BenCodez
BenCodez marked this pull request as ready for review August 26, 2026 00:18
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for security reviews. Please try again later.

@BenCodez
BenCodez force-pushed the security/javascript-placeholder-bindings branch from fda8fa8 to 0668d44 Compare August 26, 2026 00:19

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0668d44b75

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3b4fb730b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread AdvancedCore/pom.xml
@BenCodez
BenCodez force-pushed the security/javascript-placeholder-bindings branch from 4bcd709 to 4c012e9 Compare August 26, 2026 00:58

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fed11f9f39

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c8ab21d78d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread AdvancedCore/pom.xml

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 36d75a5fd9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 04f9f6e5e1

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 26429e5ca2

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Copy link
Copy Markdown
Owner Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3561f097de

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +565 to +566
if ("([{:;,=!?&|+-*%^~<>".indexOf(previous) >= 0) {
return true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject postfix updates before treating slash as regex

When Nashorn parsing falls back but the active engine supports modern syntax, a slash following ++ or -- is treated as the start of a regex because the immediately preceding +/- is accepted here. Fresh evidence beyond the earlier division thread is obj?.x; a++ / %count% / 2: the scanner creates a false regex range, so a value such as 2.5 is inserted as 2\.5 rather than bound and the valid division expression becomes invalid. Check for postfix update operators before allowing these characters to start a regex.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Owner Author

Superseded by #303.

The replacement starts cleanly from master and establishes the security boundary before replacement: only [Javascript=...] segments present in the original configured text can execute. Placeholder output and JavaScript return values remain ordinary text and cannot manufacture another executable marker.

#303 also replaces the handwritten modern-JavaScript fallback scanner with Rhino AST classification inside known JavaScript blocks, while preserving automatic legacy placeholder handling. Java CI is green on the replacement's single-commit head.

Copy link
Copy Markdown
Owner Author

Closing this approach as superseded. The replacement will start from current master and establish JavaScript boundaries before any placeholder processing: only [Javascript=...] blocks authored in the original configuration are executable, while placeholder output remains ordinary data/text. This avoids carrying forward the parser/fallback-scanner design from #302.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants