Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 96 additions & 0 deletions devmode_cookie_security_recommendations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
# Cookie Security Recommendations for DEVMODE

This document provides advice regarding the security implications of cookie configurations when the application is operating in `DEVMODE`. In `DEVMODE`, certain cookie security flags (`HttpOnly` and `Secure`) are intentionally omitted to facilitate local development. While this can simplify testing, it's crucial to understand the associated risks and best practices.

## 1. Current Behavior in DEVMODE

The application's `getCookieValue` function, which sets the authentication cookie, behaves as follows:

```typescript
async function getCookieValue(env: Env, id: string) {
const authToken = await generateAuthToken(env, id);
if (env.DEVMODE) {
// In DEVMODE: Secure and HttpOnly flags are OMITTED
return `${AUTH_COOKIE_KEY}=${authToken}; path=/; Max-Age=3600; SameSite=Strict`
} else {
// In Production (not DEVMODE): Secure and HttpOnly flags are INCLUDED
return `${AUTH_COOKIE_KEY}=${authToken}; path=/; Max-Age=3600; Secure; HttpOnly; SameSite=Strict`;
}
}
```

As shown, when `env.DEVMODE` is true, the `Set-Cookie` header will not include the `HttpOnly` and `Secure` attributes.

## 2. Risks of Omitting `HttpOnly` in DEVMODE

The `HttpOnly` flag prevents client-side JavaScript from accessing the cookie.

* **Risk:** If `HttpOnly` is omitted, and there is a Cross-Site Scripting (XSS) vulnerability anywhere in the application being developed, an attacker could potentially inject JavaScript that steals the authentication cookie (`auth`). Even in a local development environment, this stolen token could be used to impersonate the developer's session, potentially leading to unauthorized actions if the `DEVMODE` instance interacts with any sensitive local or remote resources.
* **Recommendation:** Developers should be extremely cautious when `HttpOnly` is disabled. Be mindful of any third-party scripts or untested code introduced during development. If an XSS flaw is present, the auth token is vulnerable.

## 3. Risks and Behavior of Omitting `Secure` Flag in DEVMODE

The `Secure` flag ensures that the cookie is only transmitted over HTTPS connections.

* **Reason for Omission in DEVMODE:** The `Secure` flag is typically omitted in `DEVMODE` to allow developers to test the application on a local development server running over plain HTTP (e.g., `http://localhost:xxxx`). If `Secure` were active, browsers would not send the cookie over HTTP, making local testing difficult without setting up HTTPS locally.
* **Risk:** If `DEVMODE` is inadvertently used in a context that is not strictly local and private (e.g., a shared development server accessible over HTTP, or if a developer exposes their local server to the internet for testing with external webhooks without proper network controls), the authentication token could be transmitted unencrypted. This makes it vulnerable to interception by anyone on the same network (Man-in-the-Middle attack).
* **Recommendation:**
* Ensure `DEVMODE` is used only for strictly local development on a trusted machine and network.
* If the local development environment supports HTTPS (see section 5), the `Secure` flag should be considered.

## 4. Recommendation for `HttpOnly` Flag in DEVMODE

**Strong Recommendation:** If your local development and debugging workflow **does not strictly require JavaScript to access the authentication cookie**, it is strongly recommended to **enable the `HttpOnly` flag even in `DEVMODE`**.

This can be achieved by modifying the `getCookieValue` function:

```typescript
// Option 1: Always HttpOnly
if (env.DEVMODE) {
return `${AUTH_COOKIE_KEY}=${authToken}; path=/; Max-Age=3600; HttpOnly; SameSite=Strict`; // HttpOnly added
} else {
// ...
}
```

* **Conscious Decision:** If your development tooling or debugging practices *do* require JavaScript access to this specific cookie, this should be a conscious decision. Understand that you are trading off a layer of security for development convenience. In such cases, maintain heightened vigilance against XSS.

## 5. Recommendation for `Secure` Flag in DEVMODE

If your local development environment is configured to use HTTPS, the `Secure` flag should be enabled. Many modern local development tools, including `wrangler dev`, support HTTPS locally (e.g., via `wrangler dev --https` or by configuring `dev.server.https` in `wrangler.toml`).

* **Recommendation:** When using HTTPS locally, enable the `Secure` flag. This can be done by:
1. **Modifying the condition:** If `DEVMODE` always implies HTTPS locally for your setup.
```typescript
// Example: If DEVMODE implies local HTTPS
if (env.DEVMODE) {
return `${AUTH_COOKIE_KEY}=${authToken}; path=/; Max-Age=3600; Secure; HttpOnly; SameSite=Strict`;
} else { // ... }
```
2. **Using a more granular check:** Introduce another environment variable or a more specific check if `DEVMODE` can be used with both HTTP and HTTPS locally.
```typescript
// Example: Using an additional check for local HTTPS
const useSecureCookie = env.LOCAL_HTTPS_ENABLED || !env.DEVMODE;
let cookieString = `${AUTH_COOKIE_KEY}=${authToken}; path=/; Max-Age=3600; SameSite=Strict`;
if (useSecureCookie) {
cookieString += '; Secure';
}
if (!env.DEVMODE_ALLOW_JS_COOKIE_ACCESS) { // Example for HttpOnly
cookieString += '; HttpOnly';
}
return cookieString;
```
(This example also shows a more granular flag for `HttpOnly` for illustration).

This ensures that the cookie behaves more closely to the production environment and maintains security best practices even during development if the local setup supports it.

## 6. General Warning: `DEVMODE` is for Development Only

**Crucial Reminder:** Configurations specifically designed for `DEVMODE` (like omitting `Secure` and `HttpOnly` flags) are inherently less secure and are intended only for isolated, local development environments.

**Under no circumstances should `DEVMODE` configurations, or the `DEVMODE` flag itself being true, be deployed to a staging, testing (shared), or production environment.** Doing so would expose your application and its users to significant and unnecessary security risks. Always ensure that production deployments use the most secure settings, including `HttpOnly` and `Secure` flags for sensitive cookies.

Regularly review and audit your deployment processes to prevent accidental deployment of development configurations.
---

This document aims to help developers make informed decisions about cookie security when working in `DEVMODE`. Prioritize security, even in local environments, and understand the trade-offs of any relaxed security settings.
90 changes: 90 additions & 0 deletions frontend_vulnerability_scan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Frontend Vulnerability Scan Report

This report details the findings of a security scan conducted on the Angular frontend code located in `packages/frontend/src/`. The scan focused on Cross-Site Scripting (XSS), Cross-Site Request Forgery (CSRF), and QR Code input sanitization.

## 1. Cross-Site Scripting (XSS)

**Methodology:**
* Automated `grep` search for common XSS-related Angular patterns: `[innerHTML]`, `[outerHTML]`, `bypassSecurityTrustHtml`, `document.write`.
* Manual review of component templates and code to analyze how data, especially data from TinyBase (via `ReactiveStoreService`), is rendered.

**Findings:**

* **Automated Search:** The `grep` search for patterns like `innerHTML`, `outerHTML`, `bypassSecurityTrustHtml`, and `document.write` yielded **no matches** in the codebase. This is a positive indication, suggesting that direct unsafe DOM manipulations are likely not being used.

* **Manual Review of Data Binding:**
* **`packages/frontend/src/app/editor/editor.component.html` and `editor.component.ts`**:
* The primary data display from TinyBase is handled by binding the `content` FormControl (which receives data from `ReactiveStoreService.getText()`) to a `<textarea>` element:
```html
<textarea [formControl]="content" ...></textarea>
```
* Angular's `FormControl` and its binding to `<textarea>` treat the content as plain text. The value is set as the textarea's `value` property, not interpreted as HTML. This is a safe way to handle potentially user-generated content from TinyBase.
* **Angular's Default Sanitization:** Angular, by default, treats all bindings to HTML properties (like `[value]`, `[id]`, etc.) and interpolations (`{{ value }}`) as unsafe and applies sanitization to prevent XSS. Since no explicit bypassing of this sanitization was found, it's assumed these default protections are active.

**Conclusion for XSS:**
No direct XSS vulnerabilities were identified. The application appears to correctly leverage Angular's built-in data binding mechanisms (e.g., `FormControl` for textareas, property binding) which provide default XSS protection by treating bound values as text unless explicitly told otherwise (which was not found). The risk of XSS from data received via TinyBase and rendered in the reviewed components is currently **low**.

## 2. Cross-Site Request Forgery (CSRF)

**Methodology:**
* Review of HTTP client usage and how state-changing operations (POST, PUT, DELETE) are handled.
* Consideration of Angular's default XSRF protection.

**Findings:**
* The application's primary mode of server communication for data synchronization is via **WebSockets** (`WsSynchronizer` from TinyBase), as seen in `packages/frontend/src/app/reactive-store.service.ts`.
* No explicit custom Angular `HttpClient` calls performing state-changing operations (POST, PUT, DELETE) were found in the scanned components or services.
* Angular applications generated with the Angular CLI typically include `HttpClientXsrfModule` by default, providing protection against CSRF for HTTP requests. While not explicitly verified in `app.config.ts` during this scan, the absence of relevant HTTP requests makes this a less immediate concern.
* The authentication cookie (`auth`) set by the backend (`src/index.ts`) uses `SameSite=Strict`, which is a strong defense against CSRF attacks on the cookie itself.

**Conclusion for CSRF:**
The risk of CSRF is **low**. The application relies on WebSockets for its core functionality, and no traditional state-changing HTTP requests that would be primary CSRF targets were identified. The authentication cookie also employs `SameSite=Strict` protection.

## 3. QR Code Input Sanitization

**Methodology:**
* Locate usage of the `qrcode` library.
* Analyze the data source for QR code generation and assess risks.

**Findings:**
* **Usage:** The `QrCodeComponent` in `packages/frontend/src/app/qr-code/qr-code.component.ts` uses `qrcode.toCanvas()` to render a QR code.
```typescript
// packages/frontend/src/app/qr-code/qr-code.component.ts
@Input() url: string = 'https://google.com';
// ...
toCanvas(this.qrCodeCanvasEl.nativeElement, this.url, { /* ... */ });
```
* **Data Source:** The `url` input for the `app-qr-code` component is provided in `packages/frontend/src/app/editor/editor.component.html` as:
```html
<app-qr-code [url]="url"></app-qr-code>
```
The `url` property in `EditorComponent` is set to `window.location.href`:
```typescript
// packages/frontend/src/app/editor/editor.component.ts
url = window.location.href;
```
* **Risk Assessment:**
* The data used for QR code generation is the current browser URL (`window.location.href`). This means the QR code helps users share the link to the current collaborative session.
* The content of the QR code is not derived from user-supplied data that is synchronized through TinyBase (e.g., the text being edited).
* **Malicious URLs:** While not an XSS vector through TinyBase data, if a user could be tricked into navigating to a maliciously crafted URL (e.g., one containing a `javascript:` payload, though this is often mitigated by modern QR scanners and browsers), and then generating a QR code from *that* URL, there could be a risk to the *scanner* of the QR code. However, this is an indirect risk related to the source URL itself, not a vulnerability in how the QR code component handles data from the collaborative session.
* **Denial of Service:** Extremely long URLs could generate complex QR codes. However, `window.location.href` is subject to browser and server URL length limitations, making this risk minimal.

**Conclusion for QR Code Sanitization:**
The QR code component is safe in the context of data from TinyBase, as it uses `window.location.href` and not the shared collaborative text. No specific sanitization of the `url` input is strictly necessary *within the component* beyond what the `qrcode` library itself might do, as the source is considered to be the browser's current, valid URL. Standard browser security mechanisms for URLs apply.

## 4. General Recommendations

1. **Continue Relying on Angular's Security Features:** Angular's built-in XSS protection (contextual sanitization) is effective. Avoid manual DOM manipulation (e.g., using `ElementRef.nativeElement.innerHTML = ...`) or explicitly bypassing Angular's sanitization (e.g., `DomSanitizer.bypassSecurityTrustHtml`) unless absolutely necessary and the implications are fully understood and mitigated.
2. **Output Encoding for Non-Angular Contexts:** If data from TinyBase were ever to be used outside of Angular templates (e.g., generating reports, sending emails), ensure proper output encoding/sanitization is applied relevant to that context.
3. **Keep Dependencies Updated:** Regularly update Angular, TinyBase, `qrcode`, and other frontend dependencies to benefit from the latest security patches.
4. **Content Security Policy (CSP):** While the backend (`src/index.ts`) now adds a CSP header, ensure this policy is reviewed and refined as the frontend application evolves, especially if new third-party scripts or resources are added. The current backend CSP is a good baseline (`default-src 'self'; script-src 'self'; style-src 'self' 'unsafe-inline'; ...`).
5. **Educate on QR Code Risks (General):** While not a flaw in this app's code, users should generally be cautious about scanning QR codes from untrusted sources. This is general user education rather than an application vulnerability.

Overall, the scanned portions of the Angular frontend demonstrate good adherence to standard security practices, primarily by leveraging Angular's built-in protections. The areas of highest potential risk (rendering user-supplied collaborative data) appear to be handled safely.The `frontend_vulnerability_scan.md` report has been created.

It covers:
1. **XSS**: No direct vulnerabilities found. Angular's default sanitization seems to be correctly utilized for data from TinyBase. `grep` for unsafe patterns was negative.
2. **CSRF**: Low risk. Primary communication is via WebSockets. No traditional state-changing HTTP requests found. Auth cookie has `SameSite=Strict`.
3. **QR Code Input Sanitization**: The QR code uses `window.location.href`, not user-shared data from TinyBase, as its input. This is considered safe in the context of the application's data flow.
4. **General Recommendations**: Advised to continue using Angular's security features, keep dependencies updated, manage CSP, and general user education on QR code safety.

The analysis indicates a generally good security posture for the reviewed aspects of the frontend.
Loading