diff --git a/.github/instructions/broken-access-control-prevention.instructions.md b/.github/instructions/broken-access-control-prevention.instructions.md new file mode 100644 index 000000000..394e6cd3e --- /dev/null +++ b/.github/instructions/broken-access-control-prevention.instructions.md @@ -0,0 +1,107 @@ +--- +applyTo: '**/*.py' +--- + +# Security: Broken Access Control Prevention + +## Critical Requirement + +**NEVER treat caller-supplied ids or stored active-scope settings as authorization decisions after login.** + +Treat all of the following as untrusted authorization inputs unless the code proves otherwise: + +- `conversation_id`, `message_id`, `document_id`, `file_id`, `approval_id`, `group_id`, and `public_workspace_id` +- `activeGroupOid` and `activePublicWorkspaceOid` values loaded from user settings +- Plugin or tool-call arguments such as `user_id`, `conversation_id`, `group_id`, `public_workspace_id`, `scope_id`, and `scope_type` + +## Preferred Safe Patterns + +Use these patterns by default: + +- Revalidate personal conversation ownership with `_authorize_personal_conversation_read(...)`, `_authorize_personal_conversation_access(...)`, or an explicit owner check before reading dependent data. +- Route `activeGroupOid` writes through `update_active_group_for_user(...)`. +- Route `activePublicWorkspaceOid` writes through `update_active_public_workspace_for_user(...)`. +- Resolve active group scope through `require_active_group(...)` instead of raw settings reads in backend and plugin code. +- Resolve active public workspace scope through `require_active_public_workspace(...)` instead of raw settings reads in backend and plugin code. +- In Semantic Kernel plugins, normalize tool-call scope ids through `_resolve_authorized_scope_arguments(...)`, `_resolve_blob_location_with_fallback(...)`, or `_resolve_authorized_fact_memory_call(...)` before storage, blob, or Cosmos access. +- Prefer request-scoped authorization context such as `g.authorized_chat_context` over raw tool arguments. + +## Disallowed Patterns For New Code + +Do not add new code that does any of the following without a reviewed exception: + +- Call `update_user_settings(...)` with a literal `{"activeGroupOid": ...}` payload outside `update_active_group_for_user(...)` +- Call `update_user_settings(...)` with a literal `{"activePublicWorkspaceOid": ...}` payload outside `update_active_public_workspace_for_user(...)` +- Read `activeGroupOid` or `activePublicWorkspaceOid` directly from raw settings in backend routes or plugins when a shared validator exists +- Expose `user_id`, `conversation_id`, `group_id`, `public_workspace_id`, `scope_id`, or `scope_type` in a `@kernel_function` surface without immediately rebinding those values to the authorized request context +- Read a personal conversation by request-derived `conversation_id` and continue to message, blob, or feedback work without an explicit ownership boundary + +## Safe Examples + +```python +conversation_item = _authorize_personal_conversation_read(user_id, conversation_id) +messages = list( + cosmos_messages_container.query_items( + query=query, + partition_key=conversation_item['id'], + ) +) +``` + +```python +update_active_group_for_user(requested_active_group, user_id=user_id) +active_group_id = require_active_group(user_id) +``` + +```python +authorized_scope = self._resolve_authorized_fact_memory_call( + scope_type=scope_type, + scope_id=scope_id, + conversation_id=conversation_id, +) +``` + +## Unsafe Examples + +```python +update_user_settings(user_id, {'activeGroupOid': group_id}) +``` + +```python +active_group_id = settings.get('settings', {}).get('activeGroupOid') +``` + +```python +@kernel_function(name='unsafe_tool') +def unsafe_tool(self, user_id: str, conversation_id: str, group_id: str = ''): + return self.store.lookup(user_id=user_id, conversation_id=conversation_id, group_id=group_id) +``` + +```python +conversation_item = cosmos_conversations_container.read_item( + item=conversation_id, + partition_key=conversation_id, +) +``` + +## PR Review Checklist + +For any Python change that reads or mutates user, group, workspace, conversation, or plugin-scoped data: + +1. Identify every caller-controlled id that crosses into a data read or mutation. +2. Revalidate ownership or membership at the sensitive operation boundary, not just at route entry. +3. Use the dedicated active-scope validators instead of raw settings reads and writes. +4. Rebind plugin scope parameters to the authorized request context before storage, blob, or Cosmos access. +5. Add or update a regression test when the change touches an authorization boundary. + +## Workflow Guardrail + +This repository includes a Development PR check in `.github/workflows/broken-access-control-check.yml` backed by `scripts/check_broken_access_control.py`. + +If a reviewed exception is unavoidable, add the suppression token below near the specific line and include a justification comment: + +```text +bac-check: ignore +``` + +Use that escape hatch rarely. It is for reviewed legacy exceptions, not normal route or plugin code. \ No newline at end of file diff --git a/.github/instructions/xss-prevention.instructions.md b/.github/instructions/xss-prevention.instructions.md new file mode 100644 index 000000000..aa9d156d0 --- /dev/null +++ b/.github/instructions/xss-prevention.instructions.md @@ -0,0 +1,133 @@ +--- +applyTo: '**/*.js, **/*.html, **/*.py' +--- + +# Security: XSS Prevention and Browser Rendering + +## Critical Requirement + +**NEVER pass untrusted data into browser HTML or JavaScript execution sinks without an explicit safe boundary.** + +Treat all of the following as untrusted unless the code proves otherwise: + +- User profile fields, workspace names, group names, agent names, document titles, filenames, tags, descriptions, emails, and ids +- API response values returned from storage, Microsoft Graph, Cosmos DB, Azure AI Search, or any plugin/tool response +- Markdown, rich text, uploaded text files, generated summaries, model output, and any server-returned error string + +## Preferred Safe Patterns + +Use these patterns by default: + +- Create DOM nodes with `document.createElement(...)` +- Set untrusted text with `textContent` +- Set trusted static classes with `className` +- Use `setAttribute(...)` or `dataset` for inert data only when DOM node creation is not practical +- Attach behavior with `addEventListener(...)` +- Normalize dynamic HTTP links with a helper such as `sanitizeHttpUrl(...)` before assigning `href` or `src` +- Sanitize rendered markdown with `DOMPurify.sanitize(marked.parse(...))` before inserting HTML +- Keep static modal or card shells fully static, then populate untrusted fields with DOM APIs after creation + +## Disallowed Patterns For New Code + +Do not add new code that does any of the following with untrusted values: + +- `innerHTML`, `outerHTML`, `insertAdjacentHTML`, or jQuery `.html(...)` +- Inline event handlers such as `onclick=`, `onerror=`, `onload=`, or `setAttribute('onclick', ...)` +- Dynamic interpolation into HTML attributes such as `href`, `src`, `title`, `style`, or `data-*` +- `javascript:` URLs +- `Markup(...)` in Python on untrusted content +- Jinja `|safe` on untrusted content +- `marked.parse(...)` output rendered without `DOMPurify.sanitize(...)` + +## Safe Examples + +### JavaScript + +```javascript +const row = document.createElement('tr'); +const nameCell = document.createElement('td'); +nameCell.textContent = user.displayName || 'Unknown User'; + +const actionButton = document.createElement('button'); +actionButton.type = 'button'; +actionButton.dataset.userId = user.id || ''; +actionButton.addEventListener('click', handleUserClick); + +row.appendChild(nameCell); +row.appendChild(actionButton); +``` + +```javascript +const renderedHtml = DOMPurify.sanitize(marked.parse(markdownText || '')); +markdownContainer.innerHTML = renderedHtml; +``` + +### HTML / Jinja + +```html + +``` + +### Python + +```python +return render_template( + 'page.html', + title=page_title, + items=items, +) +``` + +## Unsafe Examples + +```javascript +row.innerHTML = `
${error.message}
+${escapeHtml(error.message || 'Unknown error')}
${conversationId}
+ Conversation ID: ${safeConversationId}
${doc.document_id}
+ ID: ${safeDocumentId}
${escapeHtml(summary.content)}
${listLabel}
No specific workspaces recorded.
'; + const emptyStateEl = document.createElement('p'); + emptyStateEl.className = 'text-muted'; + emptyStateEl.textContent = 'No specific workspaces recorded.'; + listEl.appendChild(emptyStateEl); } } diff --git a/application/single_app/static/js/chat/chat-input-actions.js b/application/single_app/static/js/chat/chat-input-actions.js index 66eaf0444..b96caf8d8 100644 --- a/application/single_app/static/js/chat/chat-input-actions.js +++ b/application/single_app/static/js/chat/chat-input-actions.js @@ -154,7 +154,7 @@ export function showFileContentPopup(fileContent, filename, isTable, fileContent