Skip to content

Commit ad07907

Browse files
Merge pull request #11 from BitcoinErrorLog/fix/phase2-high-priority-fixes
fix: Phase 2 High Priority Fixes - SDK Singleton, Capability Validation, DOMPurify
2 parents 5dbafe3 + ed71ec0 commit ad07907

File tree

12 files changed

+178
-61
lines changed

12 files changed

+178
-61
lines changed

package-lock.json

Lines changed: 30 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,19 @@
2121
"test:e2e:errors": "node run-e2e-with-error-capture.js"
2222
},
2323
"dependencies": {
24-
"@synonymdev/pubky": "latest",
24+
"@synonymdev/pubky": "^0.5.4",
25+
"@types/dompurify": "^3.0.5",
2526
"dom-anchor-text-position": "^5.0.0",
2627
"dom-anchor-text-quote": "^4.0.2",
28+
"dompurify": "^3.3.1",
2729
"pubky-app-specs": "^0.4.0",
2830
"qrcode": "^1.5.3",
2931
"react": "^18.3.1",
3032
"react-dom": "^18.3.1",
3133
"react-image-crop": "^10.1.8"
3234
},
3335
"devDependencies": {
36+
"@playwright/test": "^1.40.0",
3437
"@testing-library/jest-dom": "^6.4.2",
3538
"@testing-library/react": "^14.2.1",
3639
"@types/chrome": "^0.0.268",
@@ -47,7 +50,6 @@
4750
"tailwindcss": "^3.4.4",
4851
"typescript": "^5.5.3",
4952
"vite": "^5.3.3",
50-
"vitest": "^1.3.1",
51-
"@playwright/test": "^1.40.0"
53+
"vitest": "^1.3.1"
5254
}
5355
}

src/content/AnnotationManager.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
} from '../utils/validation';
1111
import { ANNOTATION_CONSTANTS, MESSAGE_TYPES, TIMING_CONSTANTS, UI_CONSTANTS } from '../utils/constants';
1212
import { isHTMLButtonElement } from '../utils/type-guards';
13+
import DOMPurify from 'dompurify';
1314

1415
/**
1516
* Annotation data structure
@@ -460,12 +461,14 @@ export class AnnotationManager {
460461

461462
const button = document.createElement('button');
462463
button.className = 'pubky-annotation-button';
463-
button.innerHTML = `
464+
// Use DOMPurify to sanitize HTML (defense-in-depth, even though this is static)
465+
const buttonHtml = `
464466
<svg fill="none" stroke="currentColor" viewBox="0 0 24 24">
465467
<path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M7 8h10M7 12h4m1 8l-4-4H5a2 2 0 01-2-2V6a2 2 0 012-2h14a2 2 0 012 2v8a2 2 0 01-2 2h-3l-4 4z" />
466468
</svg>
467469
Add Annotation
468470
`;
471+
button.innerHTML = DOMPurify.sanitize(buttonHtml);
469472

470473
// Position button to the right and slightly below the selection
471474
// Account for button width (approximately 140px) and add some padding
@@ -572,7 +575,8 @@ export class AnnotationManager {
572575
modal.className = 'pubky-annotation-modal';
573576
modal.onclick = (e) => e.stopPropagation();
574577

575-
modal.innerHTML = `
578+
// Sanitize modal HTML with DOMPurify
579+
const modalHtml = `
576580
<h3>Add Annotation</h3>
577581
<div class="selected-text">"${this.escapeHtml(this.currentSelection.text)}"</div>
578582
<textarea placeholder="Add your comment..." autofocus maxlength="${VALIDATION_LIMITS.COMMENT_MAX_LENGTH}"></textarea>
@@ -585,6 +589,7 @@ export class AnnotationManager {
585589
<button class="submit-btn">Post Annotation</button>
586590
</div>
587591
`;
592+
modal.innerHTML = DOMPurify.sanitize(modalHtml);
588593

589594
const textarea = modal.querySelector('textarea')!;
590595
const cancelBtn = modal.querySelector('.cancel-btn')!;
@@ -920,18 +925,29 @@ export class AnnotationManager {
920925
private handleHighlightClick(annotation: Annotation) {
921926
logger.info('ContentScript', 'Highlight clicked', { id: annotation.id });
922927

928+
// Highlight the clicked annotation on the page
923929
document.querySelectorAll(`.${this.activeHighlightClass}`).forEach((el) => {
924930
el.classList.remove(this.activeHighlightClass);
925931
});
926932

927933
const highlight = document.querySelector(`[data-annotation-id="${annotation.id}"]`);
928934
if (highlight) {
929935
highlight.classList.add(this.activeHighlightClass);
936+
// Scroll the highlight into view
937+
highlight.scrollIntoView({ behavior: 'smooth', block: 'center' });
930938
}
931939

940+
// Send message to background to open sidepanel
941+
// Must be synchronous to preserve user gesture context
932942
chrome.runtime.sendMessage({
933-
type: 'SHOW_ANNOTATION',
943+
type: 'OPEN_SIDE_PANEL_FOR_ANNOTATION',
934944
annotationId: annotation.id,
945+
}, () => {
946+
if (chrome.runtime.lastError) {
947+
logger.warn('ContentScript', 'Failed to send open sidepanel message', new Error(chrome.runtime.lastError.message));
948+
} else {
949+
logger.info('ContentScript', 'Sidepanel open requested', { annotationId: annotation.id });
950+
}
935951
});
936952
}
937953

src/content/DrawingManager.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { contentLogger as logger } from './logger';
22
import { compressCanvas, getRecommendedQuality, formatBytes } from '../utils/image-compression';
33
import { DRAWING_CONSTANTS, DRAWING_UI_CONSTANTS, MESSAGE_TYPES, UI_CONSTANTS } from '../utils/constants';
44
import { isHTMLInputElement } from '../utils/type-guards';
5+
import DOMPurify from 'dompurify';
56

67
export class DrawingManager {
78
private canvas: HTMLCanvasElement | null = null;
@@ -202,7 +203,7 @@ export class DrawingManager {
202203
backdrop-filter: blur(10px);
203204
`;
204205

205-
this.toolbar.innerHTML = `
206+
const toolbarHtml = `
206207
<div style="display: flex; justify-content: space-between; align-items: center; margin-bottom: 12px;">
207208
<div>
208209
<div style="font-weight: 600; margin-bottom: 2px;">Graphiti Drawing</div>
@@ -284,6 +285,7 @@ export class DrawingManager {
284285
">Save</button>
285286
</div>
286287
`;
288+
this.toolbar.innerHTML = DOMPurify.sanitize(toolbarHtml);
287289

288290
document.body.appendChild(this.toolbar);
289291

src/content/PubkyURLHandler.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import { contentLogger as logger } from './logger';
2+
import DOMPurify from 'dompurify';
23

34
export class PubkyURLHandler {
45
constructor() {
@@ -165,10 +166,12 @@ export class PubkyURLHandler {
165166

166167
const normalizedUrl = url.replace(/^pubky:(?!\/\/)/, 'pubky://');
167168
button.setAttribute('data-pubky-url', normalizedUrl);
168-
button.innerHTML = `
169+
// Sanitize button HTML with DOMPurify (synchronous import)
170+
const buttonHtml = `
169171
<span class="pubky-link-icon">🔗</span>
170172
<span>${url.length > 30 ? url.substring(0, 30) + '...' : url}</span>
171173
`;
174+
button.innerHTML = DOMPurify.sanitize(buttonHtml);
172175
fragments.push(button);
173176

174177
lastIndex = matchIndex + url.length;

src/offscreen/offscreen.ts

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,8 @@ class OffscreenHandler {
5454
try {
5555
console.log('[Graphiti Offscreen] Initializing Pubky client...');
5656

57-
const { Client } = await import('@synonymdev/pubky');
58-
this.client = new Client();
57+
const { getPubkyClientAsync } = await import('../utils/pubky-client-factory');
58+
this.client = await getPubkyClientAsync();
5959
this.isInitialized = true;
6060

6161
console.log('[Graphiti Offscreen] Pubky client initialized successfully');
@@ -287,7 +287,14 @@ class OffscreenHandler {
287287
const allAnnotations = await annotationStorage.getAllAnnotations();
288288
for (const url in allAnnotations) {
289289
for (const annotation of allAnnotations[url]) {
290-
if (!annotation.postUri && annotation.author) {
290+
// Sync if: no postUri AND (has author OR we can assign current session as author)
291+
if (!annotation.postUri) {
292+
// If annotation was created while logged out, assign current session as author
293+
if (!annotation.author || annotation.author === '') {
294+
annotation.author = session.pubky;
295+
await annotationStorage.saveAnnotation(annotation);
296+
}
297+
291298
const result = await this.syncAnnotation({
292299
url: annotation.url,
293300
selectedText: annotation.selectedText,

src/profile/profile-renderer.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import { imageHandler } from '../utils/image-handler';
7+
import DOMPurify from 'dompurify';
78

89
// Inline logger for profile renderer
910
class ProfileRendererLogger {
@@ -76,9 +77,9 @@ class ProfileRenderer {
7677

7778
private async initializePubky() {
7879
try {
79-
const { Client } = await import('@synonymdev/pubky');
80-
this.pubky = new Client();
81-
rendererLogger.info('Pubky Client initialized');
80+
const { getPubkyClientAsync } = await import('../utils/pubky-client-factory');
81+
this.pubky = await getPubkyClientAsync();
82+
rendererLogger.info('Pubky Client initialized via singleton');
8283
} catch (error) {
8384
rendererLogger.error('Failed to initialize Pubky Client', error);
8485
throw new Error('Failed to initialize Pubky client');
@@ -255,16 +256,16 @@ class ProfileRenderer {
255256
}
256257
}
257258

258-
private showContent(html: string, title: string) {
259+
private async showContent(html: string, title: string) {
259260
// Update document title
260261
document.title = title;
261262

262263
// Hide loading and error
263264
this.loadingEl.classList.add('hidden');
264265
this.errorEl.classList.add('hidden');
265266

266-
// Show content
267-
this.contentEl.innerHTML = html;
267+
// Sanitize HTML from homeserver before displaying (critical security fix)
268+
this.contentEl.innerHTML = DOMPurify.sanitize(html);
268269
this.contentEl.classList.remove('hidden');
269270
}
270271

src/utils/auth-sdk.ts

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { logger } from './logger';
22
import { storage, Session } from './storage';
33
import { profileManager } from './profile-manager';
4+
import { getPubkyClientAsync } from './pubky-client-factory';
45

56
/**
67
* Pubky authentication using official @synonymdev/pubky SDK
@@ -25,7 +26,21 @@ class AuthManagerSDK {
2526
private currentAuthRequest: AuthRequest | null = null;
2627

2728
private constructor() {
28-
this.initializePubky();
29+
// Client will be initialized lazily via ensureClient()
30+
}
31+
32+
/**
33+
* Validate capabilities format before use
34+
*/
35+
private validateCapabilities(capabilities: string): string {
36+
if (!capabilities || typeof capabilities !== 'string') {
37+
throw new Error('Capabilities must be a non-empty string');
38+
}
39+
// Validate format - must start with /pub/
40+
if (!capabilities.startsWith('/pub/')) {
41+
throw new Error('Capabilities must start with /pub/');
42+
}
43+
return capabilities;
2944
}
3045

3146
static getInstance(): AuthManagerSDK {
@@ -36,28 +51,14 @@ class AuthManagerSDK {
3651
}
3752

3853
/**
39-
* Initialize Pubky client
40-
*/
41-
private async initializePubky() {
42-
try {
43-
// Dynamic import to handle the SDK
44-
const { Client } = await import('@synonymdev/pubky');
45-
this.client = new Client();
46-
logger.info('AuthSDK', 'Pubky Client initialized');
47-
} catch (error) {
48-
logger.error('AuthSDK', 'Failed to initialize Pubky Client', error as Error);
49-
throw error;
50-
}
51-
}
52-
53-
/**
54-
* Ensure Client is initialized
54+
* Ensure Client is initialized using singleton factory
5555
*/
5656
private async ensureClient(): Promise<Client> {
5757
if (!this.client) {
58-
await this.initializePubky();
58+
this.client = await getPubkyClientAsync();
59+
logger.info('AuthSDK', 'Pubky Client initialized via singleton');
5960
}
60-
return this.client!;
61+
return this.client;
6162
}
6263

6364
/**
@@ -69,8 +70,11 @@ class AuthManagerSDK {
6970

7071
const client = await this.ensureClient();
7172

72-
// Create auth request with capabilities
73-
this.currentAuthRequest = client.authRequest(RELAY_URL, REQUIRED_CAPABILITIES);
73+
// Validate capabilities before creating auth request
74+
const validatedCapabilities = this.validateCapabilities(REQUIRED_CAPABILITIES);
75+
76+
// Create auth request with validated capabilities
77+
this.currentAuthRequest = client.authRequest(RELAY_URL, validatedCapabilities);
7478

7579
// Get authorization URL (pubkyauth:// URL)
7680
const authUrl = this.currentAuthRequest.url();

src/utils/image-handler.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ export class ImageHandler {
2424

2525
private async ensureClient(): Promise<any> {
2626
if (!this.client) {
27-
const { Client } = await import('@synonymdev/pubky');
28-
this.client = new Client();
27+
const { getPubkyClientAsync } = await import('./pubky-client-factory');
28+
this.client = await getPubkyClientAsync();
2929
}
3030
return this.client;
3131
}

src/utils/profile-manager.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,12 @@ export class ProfileManager {
2828
}
2929

3030
/**
31-
* Initialize the Pubky client
31+
* Initialize the Pubky client using singleton factory
3232
*/
3333
private async ensureClient(): Promise<any> {
3434
if (!this.client) {
35-
const { Client } = await import('@synonymdev/pubky');
36-
this.client = new Client();
35+
const { getPubkyClientAsync } = await import('./pubky-client-factory');
36+
this.client = await getPubkyClientAsync();
3737
}
3838
return this.client;
3939
}

0 commit comments

Comments
 (0)