DOC-5922 Markdown checklist format #2351
Conversation
![jit-ci[bot]](https://avatars.githubusercontent.com/in/142441?s=60&v=4){kind=small}
There was a problem hiding this comment.
❌ Jit has detected 2 important findings in this PR that you should review.
The findings are detailed below as separate comments.
It’s highly recommended that you fix these security issues before merge.
Repository Risks:
- Internally Accessible: Accessible only within the internal network, reducing exposure to external threats but still requiring proper controls.
- Database Integration: Connects to a database, often involving sensitive data that must be securely managed.
- High Severity Findings: Indicates that the resource has high severity security findings that need attention.
- Production: Critical as it operates in a live production environment, directly impacting users and business operations.
Repository Context:
graph LR
GitHub$Repository_U23_redis/docs["GitHub Repository<br/>redis/docs"]:::GitHub$Repository
Team_U23_docs_U2D_reviewers["Team<br/>docs-reviewers"]:::Team
Team_U23_client_U2D_docs["Team<br/>client-docs"]:::Team
Team_U23_core_U2D_team["Team<br/>core-team"]:::Team
Team_U23_docs["Team<br/>docs"]:::Team
DBIntegration_U23_redis["DBIntegration<br/>redis"]:::DBIntegration
GitHub$Actions_U23_cleanup_pr_folders_U2E_yaml["GitHub Actions<br/>cleanup_pr_folders.yaml"]:::GitHub$Actions
Team_U23_docs_U2D_reviewers -- "Owns" --> GitHub$Repository_U23_redis/docs
Team_U23_client_U2D_docs -- "Owns" --> GitHub$Repository_U23_redis/docs
Team_U23_core_U2D_team -- "Owns" --> GitHub$Repository_U23_redis/docs
Team_U23_docs -- "Owns" --> GitHub$Repository_U23_redis/docs
GitHub$Repository_U23_redis/docs -- "Is accessible to" --> DBIntegration_U23_redis
GitHub$Repository_U23_redis/docs -- "Has" --> GitHub$Actions_U23_cleanup_pr_folders_U2E_yaml
|
|
||
| // Add counters | ||
| const countersDiv = document.createElement('div'); | ||
| countersDiv.innerHTML = ` |
There was a problem hiding this comment.
Security control: Static Code Analysis Semgrep Pro
Javascript.Browser.Security.Insecure-Document-Method.Insecure-Document-Method
User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities
Severity: HIGH
Fix suggestion:
This fix suggestion was generated by Jit. Please note that the suggestion might not always fit every use case. It is highly recommended that you check and review it before merging.
Suggestion guidelines
This remediation replaces the usage of insecure methods like 'innerHTML', 'outerHTML' or 'document.write' with a safer alternative, 'textContent'. The code will now use 'textContent' to safely set or update the content without putting your application at risk of XSS attacks.
| countersDiv.innerHTML = ` | |
| countersDiv.textContent = ` |
Why should you fix this issue?
This code introduces a vulnerability that could compromise the security of your production environment. In production, where reliability and security are paramount, even a small vulnerability can be exploited to cause significant damage, leading to unauthorized access or service disruption.
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fpIgnore and mark this specific single instance of finding as “False Positive”#jit_ignore_acceptIgnore and mark this specific single instance of finding as “Accept Risk”#jit_ignore_type_in_fileIgnore any finding of type "javascript.browser.security.insecure-document-method.insecure-document-method" in static/js/checklist.js; future occurrences will also be ignored.#jit_undo_ignoreUndo ignore command
| options.forEach(opt => { | ||
| const option = document.createElement('option'); | ||
| option.value = opt.value; | ||
| option.innerHTML = opt.label; |
There was a problem hiding this comment.
Security control: Static Code Analysis Semgrep Pro
Javascript.Browser.Security.Insecure-Document-Method.Insecure-Document-Method
User controlled data in methods like innerHTML, outerHTML or document.write is an anti-pattern that can lead to XSS vulnerabilities
Severity: HIGH
Fix suggestion:
This fix suggestion was generated by Jit. Please note that the suggestion might not always fit every use case. It is highly recommended that you check and review it before merging.
Suggestion guidelines
This remediation replaces the usage of insecure methods like 'innerHTML', 'outerHTML' or 'document.write' with a safer alternative, 'textContent'. The code will now use 'textContent' to safely set or update the content without putting your application at risk of XSS attacks.
| option.innerHTML = opt.label; | |
| option.textContent = opt.label; |
Why should you fix this issue?
This code introduces a vulnerability that could compromise the security of your production environment. In production, where reliability and security are paramount, even a small vulnerability can be exploited to cause significant damage, leading to unauthorized access or service disruption.
Jit Bot commands and options (e.g., ignore issue)
You can trigger Jit actions by commenting on this PR review:
#jit_ignore_fpIgnore and mark this specific single instance of finding as “False Positive”#jit_ignore_acceptIgnore and mark this specific single instance of finding as “Accept Risk”#jit_ignore_type_in_fileIgnore any finding of type "javascript.browser.security.insecure-document-method.insecure-document-method" in static/js/checklist.js; future occurrences will also be ignored.#jit_undo_ignoreUndo ignore command
I wouldn't have guessed this but apparently, AI loves checklists because they are easy to understand and relate to the surrounding content. We use interactive checklists in the client Production usage pages, but they're currently done with shortcodes. What AI really wants is Markdown.
I'm trying a different approach that generates the checklist in-page from a code-fenced Markdown checklist (like the ones you get on pull requests). The AI doesn't run JS, so it only sees Markdown, even in the HTML version of the page. However, the user still sees the same interactive checklist as before.
This isn't massive in itself but it seems like a nice trial of this technique - might be an easy way to include AI-friendly content in the future without spoiling the user experience.