-
Notifications
You must be signed in to change notification settings - Fork 0
fix: harden security for API client, storage, and exports (#64, #65, #66, #67, #68) #118
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,7 +8,6 @@ | |
| "storage", | ||
| "unlimitedStorage", | ||
| "cookies", | ||
| "activeTab", | ||
| "tabs", | ||
| "alarms", | ||
| "offscreen" | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -281,7 +281,11 @@ export class BulkApiService { | |
| const values = headers.map(h => { | ||
| const val = record[h]; | ||
| if (val === null || val === undefined) return ''; | ||
| const str = String(val); | ||
| let str = String(val); | ||
| // Security (#66): neutralize formula-injection payloads in upload CSV. | ||
| if (/^[=+\-@\t\r]/.test(str)) { | ||
| str = `'${str}`; | ||
|
Comment on lines
+286
to
+287
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For Bulk imports, Useful? React with 👍 / 👎. |
||
| } | ||
| if (str.includes(',') || str.includes('"') || str.includes('\n')) { | ||
| return `"${str.replace(/"/g, '""')}"`; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,10 +55,15 @@ export class StorageService { | |
| return orgs[orgId] ?? null; | ||
| } | ||
|
|
||
| /** Save or update an org */ | ||
| /** Save or update an org (tokens stripped — stored only in session storage) */ | ||
| async saveOrg(org: SalesforceOrg): Promise<void> { | ||
| const orgs = await this.getOrgs(); | ||
| orgs[org.orgId] = org; | ||
| // Security (#64): never persist tokens to local storage. | ||
| // Tokens live only in chrome.storage.session via setSessionToken(). | ||
| const { accessToken, tokenExpiresAt, ...safeOrg } = org; | ||
| void accessToken; | ||
| void tokenExpiresAt; | ||
| orgs[org.orgId] = safeOrg as SalesforceOrg; | ||
|
Comment on lines
+63
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the service worker is evicted during a Bulk job, Useful? React with 👍 / 👎. |
||
| await this.setLocal(STORAGE_KEYS.ORGS, orgs); | ||
| } | ||
|
|
||
|
|
||
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.
The in-page
PanelRootconstructsSfApi('content')and rendersSettingsScreen, whose Import Data action callsimportUserData. Consequently every backup restore initiated from that panel now returnsUNAUTHORIZED_SOURCE, although the action remains visible and previously worked. Either authorize this operation using the sender's verified extension context or remove/route the panel action to an authorized surface.Useful? React with 👍 / 👎.