Skip to content

Add Express.js and WebSocket security skills#664

Open
Ayush7614 wants to merge 2 commits into
usestrix:mainfrom
Ayush7614:feat/express-websocket-skills
Open

Add Express.js and WebSocket security skills#664
Ayush7614 wants to merge 2 commits into
usestrix:mainfrom
Ayush7614:feat/express-websocket-skills

Conversation

@Ayush7614

@Ayush7614 Ayush7614 commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds two skills explicitly listed in the skills README but missing from the repo:

  • frameworks/express — Express.js middleware ordering, router auth gaps, prototype pollution, NoSQL injection, CSRF, multer uploads
  • protocols/websocket — Handshake auth, CSWSH/origin validation, subscription IDOR, Socket.io namespaces, GraphQL subscriptions

Why these skills

Verification

uv run python -c "
from strix.skills import get_all_skill_names, load_skills, validate_requested_skills
for s in ['express', 'websocket']:
    assert s in get_all_skill_names()
    assert validate_requested_skills([s]) is None
    assert '## Validation' in load_skills([s])[s]
print('OK')
"

Test plan

  • Skill discovery and validation passes locally
  • Maintainer review of technical accuracy

Fill framework and protocol gaps explicitly listed in the skills README.
@greptile-apps

greptile-apps Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds two new security skill playbooks. The main changes are:

  • Added an Express.js framework skill for middleware, auth, CSRF, CORS, uploads, and injection testing.
  • Added a WebSocket protocol skill for handshake auth, Origin checks, CSWSH, subscription IDOR, and message authorization.
  • Filled gaps for skills already referenced by the skills README.

Confidence Score: 4/5

The new skills are close, but two security-guidance paths need fixes before merging.

  • Express CSRF guidance can direct users toward deprecated middleware.
  • WebSocket CSWSH guidance can misclassify tests when SameSite cookies change handshake behavior.
  • The new skill files otherwise match the existing discovery and layout conventions.

strix/skills/frameworks/express.md; strix/skills/protocols/websocket.md

Security Review

The new skills are security guidance. Two guidance issues need correction: the Express CSRF section points reviewers to deprecated middleware, and the WebSocket CSWSH section omits the SameSite cookie state needed to judge exploitability.

Important Files Changed

Filename Overview
strix/skills/frameworks/express.md Adds the Express.js skill; discovery conventions look correct, but the CSRF guidance should stop recommending deprecated csurf.
strix/skills/protocols/websocket.md Adds the WebSocket skill; discovery conventions look correct, but the CSWSH guidance needs SameSite cookie nuance.
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
strix/skills/frameworks/express.md:112
**Deprecated CSRF Middleware Recommendation**

This line names `csurf` as the Express CSRF control to look for, but that package is deprecated and unmaintained. A reviewer following this skill can tell teams to add an unsupported CSRF dependency instead of a maintained synchronizer-token or double-submit implementation, leaving the app on a mitigation path that will not receive fixes.

```suggestion
- `express-session` + cookie auth on POST/PUT/DELETE without a maintained synchronizer-token or double-submit CSRF implementation
```

### Issue 2 of 2
strix/skills/protocols/websocket.md:66-68
**SameSite CSWSH State Missing**

This CSWSH description assumes the victim browser sends cookies on a cross-site WebSocket handshake. With `SameSite=Lax` or `Strict`, modern browsers usually withhold those cookies for this handshake, so the listed PoC can become a false positive; SameSite still does not replace Origin validation for same-site subdomain or legacy-client cases.

Reviews (1): Last reviewed commit: "Add Express.js and WebSocket security sk..." | Re-trigger Greptile

Comment thread strix/skills/frameworks/express.md Outdated
### CSRF

Express has no built-in CSRF protection.
- `express-session` + cookie auth on POST/PUT/DELETE without `csurf` or double-submit token

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.

P1 security Deprecated CSRF Middleware Recommendation

This line names csurf as the Express CSRF control to look for, but that package is deprecated and unmaintained. A reviewer following this skill can tell teams to add an unsupported CSRF dependency instead of a maintained synchronizer-token or double-submit implementation, leaving the app on a mitigation path that will not receive fixes.

Suggested change
- `express-session` + cookie auth on POST/PUT/DELETE without `csurf` or double-submit token
- `express-session` + cookie auth on POST/PUT/DELETE without a maintained synchronizer-token or double-submit CSRF implementation
Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/skills/frameworks/express.md
Line: 112

Comment:
**Deprecated CSRF Middleware Recommendation**

This line names `csurf` as the Express CSRF control to look for, but that package is deprecated and unmaintained. A reviewer following this skill can tell teams to add an unsupported CSRF dependency instead of a maintained synchronizer-token or double-submit implementation, leaving the app on a mitigation path that will not receive fixes.

```suggestion
- `express-session` + cookie auth on POST/PUT/DELETE without a maintained synchronizer-token or double-submit CSRF implementation
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +66 to +68

Cross-Site WebSocket Hijacking: victim browser opens WS to target with victim's cookies because server doesn't validate `Origin`.

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.

P1 security SameSite CSWSH State Missing

This CSWSH description assumes the victim browser sends cookies on a cross-site WebSocket handshake. With SameSite=Lax or Strict, modern browsers usually withhold those cookies for this handshake, so the listed PoC can become a false positive; SameSite still does not replace Origin validation for same-site subdomain or legacy-client cases.

Prompt To Fix With AI
This is a comment left during a code review.
Path: strix/skills/protocols/websocket.md
Line: 66-68

Comment:
**SameSite CSWSH State Missing**

This CSWSH description assumes the victim browser sends cookies on a cross-site WebSocket handshake. With `SameSite=Lax` or `Strict`, modern browsers usually withhold those cookies for this handshake, so the listed PoC can become a false positive; SameSite still does not replace Origin validation for same-site subdomain or legacy-client cases.

How can I resolve this? If you propose a fix, please make it concise.

- Replace deprecated csurf reference with maintained CSRF patterns
- Add SameSite caveat for cross-site WebSocket hijacking tests
@Ayush7614

Copy link
Copy Markdown
Contributor Author

cc: @rajpratham1

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