Skip to content

🔒 Fix Sensitive Data Exposure in Console Logging and Enhance Data Preservation #18

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.

Sign up for GitHub

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

Closed
hashexplaindata wants to merge 1 commit into from
Closed

🔒 Fix Sensitive Data Exposure in Console Logging and Enhance Data Preservation #18

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
14 changes: 6 additions & 8 deletions code/experiment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const CFG = Object.freeze({

// --- State Machine ---
const STATE = {
pid: Math.random().toString(36).substring(2, 15) + Math.random().toString(36).substring(2, 15),
pid: self.crypto.randomUUID(),
condition: CFG.CONDITION,
Comment on lines 18 to 20
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.crypto.randomUUID() will throw (and stop the whole experiment) in non-secure contexts and older browsers where crypto/randomUUID isn’t available. Please add a safe fallback (e.g., feature-detect and fall back to the previous random-string approach) so PID generation can’t crash initialization.

Copilot uses AI. Check for mistakes.
covariate: 0,
currentTrial: 0,
Expand Down Expand Up @@ -348,8 +348,6 @@ function init() {
showScreen(10);
executeBatchPayload();
});

console.log(`Diagnostic Engine Initialized. PID: ${STATE.pid} | Condition: ${STATE.condition}`);
}

function loadNextTrial() {
Expand Down Expand Up @@ -469,13 +467,13 @@ async function executeBatchPayload() {
await batch.commit();
onSyncSuccess();
} else {
console.warn("Firebase not detected. Payload logged to console:", STATE.results);
setTimeout(onSyncSuccess, 1500); // Simulate sync delay
localStorage.setItem(`telemetry_backup_${STATE.pid}`, JSON.stringify(STATE.results));
setTimeout(onSyncSuccess, 0); // Simulate sync delay
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setTimeout(onSyncSuccess, 0) doesn’t actually “simulate sync delay” (it’s effectively immediate). Either restore a non-zero delay or update the comment to avoid misleading future changes/debugging.

Suggested change
setTimeout(onSyncSuccess, 0); // Simulate sync delay
setTimeout(onSyncSuccess, 0); // Defer to next tick for consistent async behavior

Copilot uses AI. Check for mistakes.
}
Comment on lines 469 to 472
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Writing the backup to localStorage can itself throw (QuotaExceededError, disabled storage, Safari private mode). In the current else branch that exception would jump to catch, and then you attempt localStorage.setItem again, which could throw a second time and bubble out unhandled. Please guard localStorage writes (feature-detect + nested try/catch) and ensure the UI still transitions to a clear success/failure state even if persistence fails.

Copilot uses AI. Check for mistakes.
} catch (error) {
console.error("Critical Sync Failure:", error);
DOM.syncStatus.innerHTML = `<span style="color:#ff453a">⚠️ Sync Failed. Error: ${error.code || 'Network'}</span>`;
// Potential fallback: Save to localStorage for later recovery
localStorage.setItem(`telemetry_backup_${STATE.pid}`, JSON.stringify(STATE.results));
DOM.syncStatus.textContent = '⚠️ Network Timeout';
DOM.syncStatus.style.color = '#ff453a';
Comment on lines +474 to +476
Copy link

@gemini-code-assist gemini-code-assist bot Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In the case of a critical sync failure, the user is shown an error message but is left on a screen with no further actions, as final-actions remains hidden. This is inconsistent with the else block (when Firebase is not detected), where onSyncSuccess is called to show the final screen. To provide a consistent user experience and avoid leaving the user at a dead end, consider also displaying the final actions in the catch block. This allows the user to see their participant ID and have the option to restart the session, while still preserving the error message.

        localStorage.setItem(`telemetry_backup_${STATE.pid}`, JSON.stringify(STATE.results));
        DOM.syncStatus.textContent = '⚠️ Network Timeout';
        DOM.syncStatus.style.color = '#ff453a';
        DOM.finalActions.style.display = 'block';
        DOM.displayPid.innerText = STATE.pid;

Comment on lines 473 to +476
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch block unconditionally calls localStorage.setItem(...) without protecting against storage errors. If storage is unavailable/full, this will throw while handling an error and leave the user stuck on the sync screen. Wrap the backup write in its own try/catch (or skip it when unavailable) before updating syncStatus.

Copilot uses AI. Check for mistakes.
}
}

Expand Down
Loading