-
Notifications
You must be signed in to change notification settings - Fork 53
docs: add security note for session management #374
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdded a Security Note section to the embedded provider core README documenting that sessions contain sensitive cryptographic material. The note recommends secure storage practices, access controls, avoiding session data logging, and clearing sessions on logout or security events. Changes
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/embedded-provider-core/README.md`:
- Line 246: Replace the bold inline text "**Security Note**" with a proper
Markdown heading (for example "### Security Note") so it becomes a heading-level
element consistent with the rest of the document; update the README entry that
currently contains the bold text to use the heading syntax and ensure
surrounding spacing/newlines match other headings to resolve the linting
violation.
| // Disconnect | ||
| await provider.disconnect(); | ||
| ``` | ||
| **Security Note** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use a proper Markdown heading instead of bold text.
The "Security Note" should use a Markdown heading (e.g., ### Security Note) to maintain consistency with the rest of the document structure and resolve the linting violation.
📝 Proposed fix
-**Security Note**
+### Security Note📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| **Security Note** | |
| ### Security Note |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
246-246: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
In `@packages/embedded-provider-core/README.md` at line 246, Replace the bold
inline text "**Security Note**" with a proper Markdown heading (for example "###
Security Note") so it becomes a heading-level element consistent with the rest
of the document; update the README entry that currently contains the bold text
to use the heading syntax and ensure surrounding spacing/newlines match other
headings to resolve the linting violation.
Summary & Motivation
This PR adds an explicit security warning to the Session Management section of packages/embedded-provider-core/README.md.
Sessions contain sensitive cryptographic material, but this was not previously highlighted in the documentation. The added note helps better understand the security implications and follow best practices when implementing custom storage adapters.
How I Tested These Changes
This is a documentation-only change.
Verified that the added section renders correctly in Markdown and is placed immediately after the Session Management section.
Did you add a changeset?
No.
This PR only updates documentation and does not affect any package behavior, APIs, or interfaces.
Did you update the README files?
Yes.
The relevant README file was updated to include security guidance for session handling.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.