Skip to content

Preserve authored JavaScript boundaries before placeholder expansion - #304

Closed
BenCodez wants to merge 0 commit into
masterfrom
security/authored-javascript-boundaries
Closed

Preserve authored JavaScript boundaries before placeholder expansion#304
BenCodez wants to merge 0 commit into
masterfrom
security/authored-javascript-boundaries

Conversation

@BenCodez

Copy link
Copy Markdown
Owner

Summary

  • split configured text into literal and operator-authored [Javascript=...] segments before custom placeholders or PlaceholderAPI are expanded
  • evaluate only JavaScript segments that existed in the original configured text, so placeholder output cannot manufacture a new executable marker
  • preserve existing marker syntax and normal JavaScript such as Player.hasPermission("some.permission") without configuration migration
  • pass executable placeholder values to the JavaScript engine as bindings instead of copying their output into source
  • use Rhino's maintained AST only to classify authored JavaScript string, template, regex, and comment locations; there is no handwritten JavaScript fallback lexer
  • fail closed when placeholder-bearing JavaScript cannot be safely parsed
  • preserve the existing custom-first requirement order and PAPI-first reward-script order
  • cover messages, item text/conditional items, reward requirements, reward scripts, and the administrative JavaScript command path

Security boundary

Only [Javascript=...] regions present in the original configured value are executable. Custom-placeholder and PlaceholderAPI output outside those regions is ordinary text and any generated marker is neutralized.

Inside an authored JavaScript region:

  • expression placeholders become engine bindings
  • string/template/regex placeholders retain their existing literal semantics with context-specific escaping
  • placeholders inside comments remain comments
  • unsupported placeholder-bearing syntax is rejected instead of entering a heuristic fallback parser

Compatibility

No manual JavaScript configuration changes are intended. Existing direct API expressions and existing %placeholder% / {placeholder} usage remain automatic. The intentionally removed behavior is allowing placeholder output to create a new executable JavaScript marker.

This supersedes #302 with the boundary-first design discussed there.

@chatgpt-codex-connector

Copy link
Copy Markdown

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

@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: 3edf4cece6

ℹ️ 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".

import java.util.regex.Pattern;

import org.bukkit.OfflinePlayer;
import org.mozilla.javascript.CompilerEnvirons;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Add Rhino to the committed Maven build

Add the Rhino dependency and shading relocation directly to AdvancedCore/pom.xml. This commit imports org.mozilla.javascript classes, but the POM contains no Rhino dependency; the only dependency insertion is deferred to a workflow that runs solely on security/authored-javascript-boundaries. Consequently, this commit cannot compile on a normal checkout, PR branch, or after being merged elsewhere, and the packaged plugin would not contain the parser.

Useful? React with 👍 / 👎.

Comment on lines +93 to +95
git reset --soft origin/master
git commit -m "Preserve authored JavaScript boundaries"
git push --force origin HEAD:security/authored-javascript-boundaries

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Avoid force-rewriting the source branch

Replace this self-modifying workflow with committed source changes. Every non-bot push to the named branch resets the checkout to origin/master and then force-pushes the reconstructed commit without a lease, so concurrent or newer branch updates can be overwritten by a run based on an older checkout. This is confirmed by git push -h, which describes --force as “force updates,” while --force-with-lease verifies the expected old ref.

Useful? React with 👍 / 👎.

@BenCodez BenCodez closed this Aug 28, 2026
@BenCodez
BenCodez force-pushed the security/authored-javascript-boundaries branch from 3edf4ce to 22c732f Compare August 28, 2026 03:17
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