Skip to content

CSTI alpha active scan rule#7470

Draft
NabilKara wants to merge 1 commit into
zaproxy:mainfrom
NabilKara:csti_addon
Draft

CSTI alpha active scan rule#7470
NabilKara wants to merge 1 commit into
zaproxy:mainfrom
NabilKara:csti_addon

Conversation

@NabilKara

Copy link
Copy Markdown

This is a draft PR to gather feedback from the team. I think I've implemented the core functions of this alpha rule.
For an overview, check my blog post: https://nabilkara.github.io/posts/tools/csti-zap/

@github-actions

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@NabilKara

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

@psiinon

psiinon commented Jun 20, 2026

Copy link
Copy Markdown
Member

Logo
Checkmarx One – Scan Summary & Detailsbd028a69-7a1b-4d84-b91f-3bfa0e6966a8

Great job! No new security vulnerabilities introduced in this pull request


Use @Checkmarx to interact with Checkmarx PR Assistant.
Examples:
@Checkmarx how are you able to help me?
@Checkmarx rescan this PR

@kingthorin

Copy link
Copy Markdown
Member

A few quick observations.

  • All the unnecessary changes (ex: moving simpleexample) should be removed.
  • Any text returned to the user (ex, alert details) should be internationalized and use Constant.messages.getString with substitution instead of literal string with concatenation.
  • There seems to be a bunch of URL utilities that are unnecessary if you simple leverage the Apache URI classes/values that exist. Similar (perhaps) for HTML parsing/analysis if you use the Source object passed into the scan method.
  • You should collapse it all into a single commit (and force push), the development history isn't really relevant when merged upstream.
  • Instead of merging main repeatedly you should rebase on upstream/main and force push.

@kingthorin

Copy link
Copy Markdown
Member

Also use imports instead of fully qualified class access, ex:
java.util.Locale> Use an import and then in code Locale.ROOT
java.util.List > Use an import and then in code List.of("a", "b")

@NabilKara

Copy link
Copy Markdown
Author

Understandable, thank you very much for your remarks.

@kingthorin

Copy link
Copy Markdown
Member

That's just quick hits, it'll need a much more detailed review. I just wanted to give you something to move along over the weekend if you choose to. The others probably won't reply until the work week, and even then it'll depend on workload/priorities.

Thanks for your interest and tackling this!!

@NabilKara

NabilKara commented Jun 20, 2026

Copy link
Copy Markdown
Author

If you don't mind I will open a new draft PR from another branch when I finish tackling all the given remarks , only this time ofc . This just makes it easier for me.

@kingthorin

kingthorin commented Jun 21, 2026

Copy link
Copy Markdown
Member

You can always work in a different local branch, then delete this one locally (when you're sure), recreate it locally from the branch you've cleaned up and push to the same branch as this PR. (Force push)

So like call this a check it out locally as b, clean it up. Once you're sure/happy. Delete a. Checkout from b creating a, force push it to the same branch on your fork/origin a, PR gets updated.

@NabilKara NabilKara force-pushed the csti_addon branch 2 times, most recently from 7397f9d to 09df9a0 Compare June 21, 2026 12:09
Co-authored-by: Beraoud Abdelkhalek <89158254+beraoudabdelkhalek@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants