docs(plugin-dev): add Plugin Team security directives by dhruvang21 · Pull Request #129 · WordPress/agent-skills

dhruvang21 · 2026-10-23T12:25:26Z

Summary

Closes #87.

Expands the wp-plugin-development security guidance with the Plugin Team security directives. Each rule is marked Prohibited, Required, or an Exception, and every exception needs a stated functional reason.

references/security.md: adds a checklist and covers all six groups from the issue: request authorization and REST permissions; sanitization vs. allowlisting; output and SQL; code execution, uploads, and files; external services and credentials; lifecycle safeguards. Includes safe/unsafe examples for REST permissions, dynamic option names, SQL identifiers, file writes and deletes, and shortcode output.
SKILL.md: step 4 now points to the checklist as part of the normal workflow.

Added during review, beyond the issue's list:

PHP object injection: no untrusted unserialize(), no side effects in magic methods, phar:// paths.
A "Chained vulnerabilities" section, for example a missing capability check plus stored XSS leading to admin takeover.
Fail-closed checks; is_admin() is not authorization; ABSPATH guard; command injection; SSRF (wp_safe_remote_*); sslverify; wp_safe_redirect().

Policy topics (readme disclosure, consent, remote code) link to wp-plugin-directory-guidelines instead of repeating it. Nothing from the scanner's implementation is included; all guidance comes from public WordPress documentation.

Testing

node shared/scripts/validate-skills.mjs passes.
All PHP examples pass php -l.
Eval scenario plugin-security-nonce-and-capability.json not yet run manually.

🤖 Generated with Claude Code

github-actions · 2026-10-23T12:25:35Z

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: dhruvang21 <dhruvang21@git.wordpress.org>
Co-authored-by: flexseth <flexseth@git.wordpress.org>
Co-authored-by: nadeem-khan <chillopedia@git.wordpress.org>
Co-authored-by: davidperezgar <davidperez@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Props Bot commands

You can trigger Props Bot actions by commenting on this PR:

@propsbot add <username> to give props to a GitHub user. Requires write access.
@propsbot remove <username> to leave a user out of the props. Requires write access.
@propsbot refresh to refresh this list. Adding the props-bot label works too.

flexseth · 2026-10-23T12:58:08Z

@dhruvang21 - Happy to work with you on this skill, it's an important and nuanced one, and will likely need iteration.

I have a package that generates WordPress plugins from specifications, and it almost 100% of the time creates plugins that pass security guidelines for the Plugin Check Plugin (and wp.org/plugins submission.)

Working up some notes, I can get them to you later today for review.

One way you could smoke test this, is to use the agent skill to scaffold a plugin that has intentional security issues in WordPress Studio, and then use the PCP to see if your plugin passes security checks.

Studio Code / Claude prompt

I want you to use the agent skill at PR, make sure to install the WordPress agent skills first, they will help with all purpose plugin development.

You will first research Plugin Security for WordPress using official documentation from wordpress.org. Then, you will create a SECURITY.md file that has a checklist of all security checks from the WordPress Plugin Check Plugin (PCP).

Once the checklist is created, make a plugin that has a security vulnerability for each item on the checklist in SECURITY.md. There should be a 1-1 correlation between security checks and security issues when the Plugin Check Plugin is ran against the plugin. Name the plugin vulnerable-wordpress-plugin

In the plugin root, add a README.md that lists security checks and why the plugin should fail.

The last line can be useful for creating test cases to run against the PCP CLI.

dhruvang21 · 2026-10-23T13:39:05Z

Thanks @flexseth! Happy to iterate on this together, and I’d welcome your feedback.

The smoke test is a good idea. One adjustment, though: I don’t think a strict 1:1 mapping between checklist items and PCP failures will hold. Many directives in this PR are context-dependent, and static checks can’t reliably catch issues like whether a permission_callback enforces the right capability, a sanitized value being used as an option or table name, object-injection gadgets, or chained vulnerabilities. Issue #87 grew out of exactly this gap.

I’d suggest splitting the test into two parts:

PCP-detectable subset (escaping, nonce verification, unsanitized input, unprepared SQL, direct file access, etc.): Build a deliberately vulnerable plugin, confirm that PCP flags each issue, then use the skill to fix them and rerun PCP to verify that it comes back clean. The fix it with the skill step is what actually validates the skill’s effectiveness.
Context-sensitive directives: Cover these through eval scenarios under eval/scenarios/ and manual review, since a passing PCP run alone doesn’t prove these cases are handled correctly.

Also, if you could tag a few folks from the plugin teams to review and test this, that would be really helpful. Their practical experience would help validate the approach, catch gaps we might miss, and ensure we’re heading in the right direction.

flexseth · 2026-10-24T19:47:05Z

75a7638 is one very large change set and difficult to review. Do you think you can restructure the PR as a series of commits that addresses the checklist items?

workflow

Plan mode
A good workflow is to have Claude look at requirements, and create a plan. The plan should provide links to source material (docs) and code examples - in this case bad code vs. correct code.

Manual mode
Then, go into manual mode (not auto) and iteratively work through the plan in phases. Each time you complete a phase, have Claude check it off the plan, commit, push, and review it yourself. "Review it" meaning: Check the documentation versus the changes made on each commit, and personally verify each commit. By a human.

consumable commits
The commit should address what changed and why it changed. Planning, and breaking the PR down into more meaningful steps will help the LLM produce better commit messages for changes.

providing context
Looking at the PR, it's quite difficult to determine what changes were made to address the requirements from the Security Team. Break the PR into more manageable sections helps to alleviate this.

more on plan mode..
Using plan mode will also allow Claude to analyze the problem and determine the "Whats", "Whys", and "Hows" - which will be used in each commit message to provide context and explain the problem solving.

Delivery: Detailed commit messages and more commits will make the PR easier to consume and review.

Evals

Regarding:

Context-sensitive directives: Cover these through eval scenarios under eval/scenarios/ and manual review

Could you elaborate on this? I don't see any evals added with the PR...

flexseth · 2026-10-24T21:08:04Z

+```php
+// Unsafe: sanitize_key() keeps the name well-formed, but the request still
+// chooses which option is overwritten (users_can_register, default_role, ...).
+$name = sanitize_key( wp_unslash( $_POST['option'] ?? '' ) );
+update_option( $name, sanitize_text_field( wp_unslash( $_POST['value'] ?? '' ) ) );

- never process the entire `$_POST` / `$_GET` array; read explicit keys
- use `wp_unslash()` before sanitizing when needed
- use prepared statements for SQL; avoid interpolating user input into queries
+// Safe: the request can only pick from a fixed map.
+$options = array(
+	'color'  => 'myplugin_color',
+	'layout' => 'myplugin_layout',
+);
+$key = sanitize_key( wp_unslash( $_POST['setting'] ?? '' ) );
+if ( ! isset( $options[ $key ] ) ) {
+	wp_send_json_error( null, 400 );
+}
+update_option( $options[ $key ], sanitize_text_field( wp_unslash( $_POST['value'] ?? '' ) ) );
+```


Unclear which rule this code applies to

flexseth · 2026-10-24T21:12:02Z

+- Validate and sanitize input before storing it, displaying it, or passing it to a function whose behavior you don't control (hooks, third-party APIs).
+- Never process a whole `$_POST` / `$_GET` / `$_REQUEST` array; read explicit keys and apply `wp_unslash()` before sanitizing.
+- Sanitize a nonce before passing it to `wp_verify_nonce()`: `sanitize_text_field( wp_unslash( $_POST['_wpnonce'] ?? '' ) )`. `check_admin_referer()` and `check_ajax_referer()` read the request themselves.
+- Treat input as unsafe in sensitive positions even after sanitization. A fixed allowlist, strict comparison (`in_array( $v, $allowed, true )`), or hardcoded map must decide:


What does "sensitive positions" mean?

flexseth · 2026-10-24T21:29:53Z

+- Treat table names, column names, `ORDER BY` keys, and other identifiers from the request as unsafe unless they come from an allowlist. Pass allowlisted identifiers through `%i` (WordPress 6.2+). If the plugin's `Requires at least` is below 6.2, interpolate only the allowlisted constant value.
+- Prefer `$wpdb->insert()` / `$wpdb->update()` / `$wpdb->delete()` with a formats array for simple writes.
+
+```php


what rule does this code snippet explain?

flexseth · 2026-10-24T21:31:35Z

+
+**Prohibited**
+
+- Features that let users insert arbitrary PHP, JavaScript, or CSS to be executed or output on the site.


this line is marked as a bullet point, but appears to be a section summary?

flexseth · 2026-10-24T21:54:24Z

+- Keep magic methods free of side effects. `__wakeup()`, `__destruct()`, and `__toString()` must not delete or write files, include paths, or call callbacks stored in properties; otherwise any deserialization bug elsewhere on the site can use the class as a gadget. Classes that hold paths, callbacks, or credentials and are never meant to be serialized should refuse it:
+
+  ```php
+  public function __wakeup() {
+  	throw new \LogicException( 'This class cannot be unserialized.' );
+  }
+  ```


Starting point of a good pattern: Bulleted item with immediate example after.

Code example could show the "not allowed" version of the code, such as the other code samples

flexseth · 2026-10-24T21:55:56Z

+```php
+// Unsafe: persistent data in the plugin folder, with a request-chosen file name.
+file_put_contents( plugin_dir_path( __FILE__ ) . $_POST['name'] . '.php', $data );
+
+// Safe: plugin subdirectory of uploads, fixed name, non-executable format.
+$uploads = wp_upload_dir();
+$dir     = trailingslashit( $uploads['basedir'] ) . 'myplugin';
+if ( $uploads['error'] || ! wp_mkdir_p( $dir ) ) {
+	return new WP_Error( 'myplugin_storage', __( 'Storage is unavailable.', 'myplugin' ) );
+}
+file_put_contents( $dir . '/export.json', wp_json_encode( $data ) );
+
+// Safe delete: target must resolve inside the plugin's directory.
+$base   = realpath( $dir );
+$target = realpath( $dir . '/' . sanitize_file_name( $requested_file ) );
+if ( false === $base || false === $target || 0 !== strpos( $target, $base . DIRECTORY_SEPARATOR ) ) {
+	return new WP_Error( 'myplugin_invalid_path', __( 'Invalid file.', 'myplugin' ) );
+}
+wp_delete_file( $target );
+```


Add or move section heading with relevant security guideline directly above this code.

flexseth · 2026-10-24T21:59:51Z

+**Exceptions** (each needs a stated functional reason)
+
+- Drop-ins such as `object-cache.php` or `advanced-cache.php` for caching plugins, written only on an explicit administrator action and removed when the feature is turned off.
+- Backup archives, protected from direct download (unguessable names, deny rules, or storage outside the web root).
+- Temporary files from `wp_tempnam()`, deleted when the operation ends.
+- Root-level files that an integration requires (for example, a service verification file), created only on an explicit administrator action.
+- SVG uploads, only when sanitized server-side by a maintained sanitizer.


Exception requires a "stated functional reason".

Add why these exceptions are OK. Give examples, skills should have a deterministic value.

How will the harness know when to use an exception to the rule? Don't leave the AI to guess.
The harness should be able to infer from instructions when and why to use the exception.

flexseth · 2026-10-24T22:05:11Z

+
+- Document every fixed third-party service the plugin contacts in the plugin readme: what the service is, what it is used for, what data is sent and when, and links to its Terms of Service and/or Privacy Policy. For the directory consent and SaaS rules, see the `wp-plugin-directory-guidelines` skill (Guidelines 6–8).
+- Use `wp_safe_remote_get()` / `wp_safe_remote_post()` when a URL comes from input or settings. They reject local and private-network hosts, which blocks server-side request forgery.
+- Never disable TLS verification (`'sslverify' => false`) in shipped code.


What is "shipped code?"

Much of the agent skills' work may happen in dev. How does the AI know where the code lives or is intended for?

This requirement probably should be added as an eval like https://github.com/WordPress/agent-skills/blob/trunk/eval/scenarios/plugin-security-nonce-and-capability.json

flexseth · 2026-10-24T22:11:52Z

+
+**Required**
+
+- Document every fixed third-party service the plugin contacts in the plugin readme: what the service is, what it is used for, what data is sent and when, and links to its Terms of Service and/or Privacy Policy. For the directory consent and SaaS rules, see the `wp-plugin-directory-guidelines` skill (Guidelines 6–8).


Context bloat: Link directly to file instead of loading skill.

The wp-plugin-directory-guidelines/references directory has a file with relevant information. Link there.

flexseth · 2026-10-24T22:12:44Z

+Upstream reference:
+
+- https://make.wordpress.org/core/2026/03/24/introducing-the-ai-client-in-wordpress-7-0/
+


add relevant upstream references for this section. It covers a lot of platform surfaces.

flexseth · 2026-10-24T22:17:24Z

+## Chained vulnerabilities
+
+Assess each weakness by what it unlocks in combination, not in isolation. A "low-severity" gap is often one link in a full site takeover:
+
+- **Missing capability check → stored XSS → admin takeover.** A subscriber can save a value the plugin later prints unescaped in wp-admin. The script runs in an administrator's session, can read any nonce on the page, and can create an administrator, edit plugin files, or install a plugin. Nonces and capability checks don't stop it, because the request comes from a real administrator. Fix both links: authorize the write *and* escape the output.
+- **Arbitrary option update → privilege escalation.** An unrestricted `update_option()` lets an attacker set `users_can_register` and `default_role` to `administrator`, then register. Fix: authorize, and allowlist the option name.
+- **Object injection → gadget chain → file write → code execution.** Untrusted `unserialize()` plus any loaded class whose `__destruct()` or `__wakeup()` writes or deletes files is enough. Fix: no untrusted deserialization, and no side effects in magic methods.
+- **Path traversal or unrestricted upload → executable file in the web root.** Fix: plugin-owned directory, fixed names, path validation, no executable types.


this section has explanations that read clearly.

flexseth · 2026-10-24T22:19:40Z

-* For `wp_ajax_nopriv_*`, assume unauthenticated attacker-controlled traffic.
-* Return JSON with `wp_send_json_success()` or `wp_send_json_error()`.
-* Do not expose private data from AJAX responses.
+When fixing a finding, check what else reaches the same data, sink, or file, and close every link, not just the one reported.


dangling statement. provide context. Should "sink" be "sync?"

Could possibly use a section intro or more explanation.

flexseth · 2026-10-24T22:21:15Z

+- [ ] Every privileged action, sensitive read, or persistent write checks authorization *and* CSRF protection.
+- [ ] Every REST route and ability has a `permission_callback` that resolves and enforces the right capability.
+- [ ] Security checks fail closed: their return values are checked, and the request stops when they fail.
+- [ ] Input is read by explicit key, unslashed, validated, and sanitized for its type.
+- [ ] Request values that select names, paths, callbacks, capabilities, or targets go through a fixed allowlist or map.
+- [ ] Output is escaped late, including shortcode, dynamic block, and filter callback output.
+- [ ] SQL uses `$wpdb->prepare()` for values and allowlisted identifiers.
+- [ ] No code execution, executable uploads, or writes outside the plugin's uploads subdirectory.
+- [ ] No `unserialize()` / `maybe_unserialize()` of untrusted data; magic methods have no dangerous side effects.
+- [ ] Each weakness was assessed for what it unlocks when chained with others, not in isolation.
+- [ ] External services are documented in the readme; credentials come from platform APIs.
+- [ ] No global sessions, global PHP config changes, or silent plugin/theme/update changes.


Checklist should probably be created as individual evals or one large plugin security eval, whichever requires less compute.

I'm not very familiar with evals, but this seems like eval territory.

Checklist is given, but the context of the checklist items is not provided in the relevant sections in the rest of the spec.

Group checklist items by section, and add the checklist item as a brief intro per section.

This is part of a greater organizational restructure I feel this PR very much needs.

For instance, if someone wanted to get into plugin security, they aren't going to get much value with the way the PR is organized right now.

Write for humans and AI. At some point a human will be involved in the review process, or may be updating this document. Having a better organization structure will help.

flexseth · 2026-10-24T22:27:35Z

+
+For REST schemas and argument validation, see the `wp-rest-api` skill; for abilities, see the `wp-abilities-api` skill.

 Upstream reference:


add more references, link to relevant sections instead of whole doc pages. There is a lot covered in this section.

flexseth

Hey Dhruvang ,

I took a couple of hours today (4) to review this PR. It needs a lot of work. It seems like Claude interpreted the request as a new skill, and not an improvement to an existing skill. Please accept these comments and suggestions as solid constructive criticism, and in no way negative. If you want, I'd be happy to get on a Zoom call with you sometime to go over the methodology I used here.

The LLM seems to have glanced over the Agent Skills repository and cherry-picked what it thought were some best practices, but missed some very important conventions for developing good skills.

If it's OK with you, I'd like to take the directives that have been laid out, and rework the skill so it will be consumable for humans and AI. We can even look at this together during the call if you want, I have time next week.

Your contributions will be included in the PR, that I will be comfortable submitting to the maintainers for their review. And you'll get credit for contributing to the repo.

Hopefully my feedback isn't too much with the comments on the PR issue body and the code surface. They're intended to guide you towards being a better WordPress skills developer.

Let me know if that works,

Seth

docs(plugin-dev): add Plugin Team security directives

75a7638

dhruvang21 requested a review from a team as a code owner October 23, 2026 12:25

flexseth reviewed Oct 24, 2026

View reviewed changes


		Prohibited

		- Features that let users insert arbitrary PHP, JavaScript, or CSS to be executed or output on the site.


		Required

		- Document every fixed third-party service the plugin contacts in the plugin readme: what the service is, what it is used for, what data is sent and when, and links to its Terms of Service and/or Privacy Policy. For the directory consent and SaaS rules, see the `wp-plugin-directory-guidelines` skill (Guidelines 6–8).

		Upstream reference:

		- https://make.wordpress.org/core/2026/03/24/introducing-the-ai-client-in-wordpress-7-0/


		For REST schemas and argument validation, see the `wp-rest-api` skill; for abilities, see the `wp-abilities-api` skill.

		Upstream reference:

Conversation

dhruvang21 commented Oct 23, 2026

Summary

Testing

Uh oh!

github-actions Bot commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

flexseth commented Oct 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Studio Code / Claude prompt

Uh oh!

dhruvang21 commented Oct 23, 2026

Uh oh!

flexseth commented Oct 24, 2026

workflow

Evals

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flexseth Oct 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flexseth left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions Bot commented Oct 23, 2026 •

edited

Loading

flexseth commented Oct 23, 2026 •

edited

Loading

flexseth Oct 24, 2026 •

edited

Loading