Fix/auth hardening

.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.

.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
+<button type="button" class="btn btn-primary user-action-btn" data-user-id="{{ user.id }}">
+    Select
+</button>
+```
+
+### Python
+
+```python
+return render_template(
+    'page.html',
+    title=page_title,
+    items=items,
+)
+```
+
+## Unsafe Examples
+
+```javascript
+row.innerHTML = `<td>${user.displayName}</td>`;
+```
+
+```javascript
+button.setAttribute('onclick', `selectUser('${user.id}', '${user.displayName}')`);
+```
+
+```html
+<a href="javascript:${payload}">Run</a>
+```
+
+```python
+return Markup(user_supplied_html)
+```
+
+```html
+{{ user_supplied_html|safe }}
+```
+
+## Static HTML Shell Exception
+
+When a static HTML shell is genuinely simpler, it is acceptable only if:
+
+- The HTML string is fully static
+- It contains no `${...}` interpolation or dynamic concatenation
+- Untrusted values are populated afterward with `textContent`, `setAttribute(...)`, or `dataset`
+
+## PR Review Checklist
+
+For any JavaScript, HTML, or Python change that affects browser rendering:
+
+1. Identify the trust boundary for every value that reaches the browser.
+2. Prefer DOM node creation and `textContent` for untrusted text.
+3. Normalize dynamic URLs before assigning them to clickable or loadable attributes.
+4. If HTML rendering is required, document the sanitizer boundary explicitly.
+5. Add or update a regression test when untrusted data reaches a browser-rendering path.
+
+## Workflow Guardrail
+
+This repository includes a Development PR check in `.github/workflows/xss-sink-check.yml` backed by `scripts/check_xss_sinks.py`.
+
+If a reviewed exception is unavoidable, add the suppression token below near the specific line and include a justification comment:
+
+```text
+xss-check: ignore
+```
+
+Use that escape hatch rarely. It is for reviewed legacy exceptions, not for normal rendering code.

.github/workflows/broken-access-control-check.yml

@@ -0,0 +1,66 @@
+name: Broken Access Control Check
+
+on:
+  pull_request:
+    branches:
+      - Development
+    paths:
+      - 'application/**/*.py'
+      - 'scripts/check_broken_access_control.py'
+      - 'functional_tests/test_broken_access_control_guardrails_checker.py'
+      - '.github/workflows/broken-access-control-check.yml'
+      - '.github/instructions/broken-access-control-prevention.instructions.md'
+
+jobs:
+  broken-access-control-check:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+
+      - name: Get changed Python files
+        id: changed-files
+        uses: tj-actions/changed-files@v46.0.1
+        with:
+          files_yaml: |
+            bac_surface:
+              - 'application/**/*.py'
+            bac_guardrails:
+              - 'scripts/check_broken_access_control.py'
+              - 'functional_tests/test_broken_access_control_guardrails_checker.py'
+              - '.github/workflows/broken-access-control-check.yml'
+              - '.github/instructions/broken-access-control-prevention.instructions.md'
+              - 'docs/explanation/features/v0.241.022/BROKEN_ACCESS_CONTROL_PR_GUARDRAILS.md'
+
+      - name: Run Broken Access Control validation
+        env:
+          CHANGED_BAC_FILES: ${{ steps.changed-files.outputs.bac_surface_all_changed_files }}
+          GITHUB_BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          GITHUB_HEAD_SHA: ${{ github.sha }}
+        run: |
+          if [[ -z "$CHANGED_BAC_FILES" ]]; then
+            echo "No changed application files detected for Broken Access Control validation."
+            exit 0
+          fi
+
+          echo "Changed application files:"
+          printf '%s\n' "$CHANGED_BAC_FILES" | tr ' ' '\n'
+
+          python scripts/check_broken_access_control.py \
+            --base-sha "$GITHUB_BASE_SHA" \
+            --head-sha "$GITHUB_HEAD_SHA" \
+            $CHANGED_BAC_FILES
+
+      - name: Run Broken Access Control guardrail self-test (advisory)
+        if: steps.changed-files.outputs.bac_guardrails_any_changed == 'true'
+        continue-on-error: true
+        run: |
+          python functional_tests/test_broken_access_control_guardrails_checker.py

.github/workflows/xss-sink-check.yml

@@ -0,0 +1,70 @@
+name: XSS Sink Check
+
+on:
+  pull_request:
+    branches:
+      - Development
+    paths:
+      - 'application/**/*.js'
+      - 'application/**/*.html'
+      - 'application/**/*.py'
+      - 'scripts/check_xss_sinks.py'
+      - 'functional_tests/test_xss_guardrails_checker.py'
+      - '.github/workflows/xss-sink-check.yml'
+      - '.github/instructions/xss-prevention.instructions.md'
+
+jobs:
+  xss-sink-check:
+    runs-on: ubuntu-latest
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - name: Set up Python
+        uses: actions/setup-python@v5
+        with:
+          python-version: '3.11'
+
+      - name: Get changed XSS-related files
+        id: changed-files
+        uses: tj-actions/changed-files@v46.0.1
+        with:
+          files_yaml: |
+            xss_surface:
+              - 'application/**/*.js'
+              - 'application/**/*.html'
+              - 'application/**/*.py'
+            xss_guardrails:
+              - 'scripts/check_xss_sinks.py'
+              - 'functional_tests/test_xss_guardrails_checker.py'
+              - '.github/workflows/xss-sink-check.yml'
+              - '.github/instructions/xss-prevention.instructions.md'
+              - 'docs/explanation/features/v0.241.021/XSS_PR_GUARDRAILS.md'
+
+      - name: Run XSS sink validation
+        env:
+          CHANGED_XSS_FILES: ${{ steps.changed-files.outputs.xss_surface_all_changed_files }}
+          GITHUB_BASE_SHA: ${{ github.event.pull_request.base.sha }}
+          GITHUB_HEAD_SHA: ${{ github.sha }}
+        run: |
+          if [[ -z "$CHANGED_XSS_FILES" ]]; then
+            echo "No changed application files detected for XSS sink validation."
+            exit 0
+          fi
+
+          echo "Changed application files:"
+          printf '%s\n' "$CHANGED_XSS_FILES" | tr ' ' '\n'
+
+          python scripts/check_xss_sinks.py \
+            --base-sha "$GITHUB_BASE_SHA" \
+            --head-sha "$GITHUB_HEAD_SHA" \
+            $CHANGED_XSS_FILES
+
+      - name: Run XSS guardrail self-test (advisory)
+        if: steps.changed-files.outputs.xss_guardrails_any_changed == 'true'
+        continue-on-error: true
+        run: |
+          python functional_tests/test_xss_guardrails_checker.py