Skip to content

Commit acee13e

Browse files
allanhaggettclaude
allanhaggett
and
claude
committed
Fix phpcs and semgrep findings in editor files
Remove blank line after opening class brace in external function classes (Moodle coding standard). Replace innerHTML assignments with safe DOM API calls (setChildren helper, textContent, createElement) to eliminate XSS risk flagged by semgrep. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 655d009 commit acee13e

File tree

5 files changed

+74
-17
lines changed

5 files changed

+74
-17
lines changed

‎amd/build/editor.min.js‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,25 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
3535
/** @type {Object} DOM element references */
3636
var dom = {};
3737

38+
/**
39+
* Safely replace all children of an element.
40+
*
41+
* @param {HTMLElement} el
42+
* @param {Array<HTMLElement|String>} children - DOM nodes or strings (inserted as text)
43+
*/
44+
var setChildren = function(el, children) {
45+
while (el.firstChild) {
46+
el.removeChild(el.firstChild);
47+
}
48+
children.forEach(function(child) {
49+
if (typeof child === 'string') {
50+
el.appendChild(document.createTextNode(child));
51+
} else {
52+
el.appendChild(child);
53+
}
54+
});
55+
};
56+
3857
/**
3958
* Check if the editor has unsaved changes.
4059
*
@@ -243,17 +262,25 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
243262
*/
244263
var loadTree = function() {
245264
Str.get_string('editor_loading', 'local_githubsync').then(function(loadingStr) {
246-
dom.tree.innerHTML = '<div class="p-3 text-center text-muted">' +
247-
'<div class="spinner-border spinner-border-sm" role="status"></div> ' +
248-
loadingStr + '</div>';
265+
var wrapper = document.createElement('div');
266+
wrapper.className = 'p-3 text-center text-muted';
267+
var spinner = document.createElement('div');
268+
spinner.className = 'spinner-border spinner-border-sm';
269+
spinner.setAttribute('role', 'status');
270+
wrapper.appendChild(spinner);
271+
wrapper.appendChild(document.createTextNode(' ' + loadingStr));
272+
setChildren(dom.tree, [wrapper]);
249273

250274
return Repository.getFileTree(state.courseid);
251275
}).then(function(result) {
252276
state.treeData = result.files;
253277
renderTree();
254278
}).catch(function(err) {
255279
Str.get_string('editor_treefailed', 'local_githubsync').then(function(msg) {
256-
dom.tree.innerHTML = '<div class="p-3 text-danger">' + msg + '</div>';
280+
var errDiv = document.createElement('div');
281+
errDiv.className = 'p-3 text-danger';
282+
errDiv.textContent = msg;
283+
setChildren(dom.tree, [errDiv]);
257284
});
258285
Notification.exception(err);
259286
});
@@ -290,11 +317,14 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
290317
Notification.addNotification({message: conflictMsg, type: 'error'});
291318
return Str.get_string('editor_reload', 'local_githubsync');
292319
}).then(function(reloadMsg) {
293-
dom.status.innerHTML = '<a href="#" id="githubsync-reload-link">' + reloadMsg + '</a>';
294-
document.getElementById('githubsync-reload-link').addEventListener('click', function(e) {
320+
var link = document.createElement('a');
321+
link.href = '#';
322+
link.textContent = reloadMsg;
323+
link.addEventListener('click', function(e) {
295324
e.preventDefault();
296325
loadFile(state.currentPath);
297326
});
327+
setChildren(dom.status, [link]);
298328
});
299329
} else if (result.success) {
300330
state.currentSha = result.newsha;
@@ -313,7 +343,7 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
313343
}).then(function() {
314344
// Restore button regardless of outcome.
315345
Str.get_string('editor_save', 'local_githubsync').then(function(saveText) {
316-
dom.saveBtn.innerHTML = saveText;
346+
dom.saveBtn.textContent = saveText;
317347
updateSaveButton();
318348
});
319349
});

‎amd/src/editor.js‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,25 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
3535
/** @type {Object} DOM element references */
3636
var dom = {};
3737

38+
/**
39+
* Safely replace all children of an element.
40+
*
41+
* @param {HTMLElement} el
42+
* @param {Array<HTMLElement|String>} children - DOM nodes or strings (inserted as text)
43+
*/
44+
var setChildren = function(el, children) {
45+
while (el.firstChild) {
46+
el.removeChild(el.firstChild);
47+
}
48+
children.forEach(function(child) {
49+
if (typeof child === 'string') {
50+
el.appendChild(document.createTextNode(child));
51+
} else {
52+
el.appendChild(child);
53+
}
54+
});
55+
};
56+
3857
/**
3958
* Check if the editor has unsaved changes.
4059
*
@@ -243,17 +262,25 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
243262
*/
244263
var loadTree = function() {
245264
Str.get_string('editor_loading', 'local_githubsync').then(function(loadingStr) {
246-
dom.tree.innerHTML = '<div class="p-3 text-center text-muted">' +
247-
'<div class="spinner-border spinner-border-sm" role="status"></div> ' +
248-
loadingStr + '</div>';
265+
var wrapper = document.createElement('div');
266+
wrapper.className = 'p-3 text-center text-muted';
267+
var spinner = document.createElement('div');
268+
spinner.className = 'spinner-border spinner-border-sm';
269+
spinner.setAttribute('role', 'status');
270+
wrapper.appendChild(spinner);
271+
wrapper.appendChild(document.createTextNode(' ' + loadingStr));
272+
setChildren(dom.tree, [wrapper]);
249273

250274
return Repository.getFileTree(state.courseid);
251275
}).then(function(result) {
252276
state.treeData = result.files;
253277
renderTree();
254278
}).catch(function(err) {
255279
Str.get_string('editor_treefailed', 'local_githubsync').then(function(msg) {
256-
dom.tree.innerHTML = '<div class="p-3 text-danger">' + msg + '</div>';
280+
var errDiv = document.createElement('div');
281+
errDiv.className = 'p-3 text-danger';
282+
errDiv.textContent = msg;
283+
setChildren(dom.tree, [errDiv]);
257284
});
258285
Notification.exception(err);
259286
});
@@ -290,11 +317,14 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
290317
Notification.addNotification({message: conflictMsg, type: 'error'});
291318
return Str.get_string('editor_reload', 'local_githubsync');
292319
}).then(function(reloadMsg) {
293-
dom.status.innerHTML = '<a href="#" id="githubsync-reload-link">' + reloadMsg + '</a>';
294-
document.getElementById('githubsync-reload-link').addEventListener('click', function(e) {
320+
var link = document.createElement('a');
321+
link.href = '#';
322+
link.textContent = reloadMsg;
323+
link.addEventListener('click', function(e) {
295324
e.preventDefault();
296325
loadFile(state.currentPath);
297326
});
327+
setChildren(dom.status, [link]);
298328
});
299329
} else if (result.success) {
300330
state.currentSha = result.newsha;
@@ -313,7 +343,7 @@ define(['local_githubsync/repository', 'core/notification', 'core/str'], functio
313343
}).then(function() {
314344
// Restore button regardless of outcome.
315345
Str.get_string('editor_save', 'local_githubsync').then(function(saveText) {
316-
dom.saveBtn.innerHTML = saveText;
346+
dom.saveBtn.textContent = saveText;
317347
updateSaveButton();
318348
});
319349
});

‎classes/external/get_file_content.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
3232
*/
3333
class get_file_content extends external_api {
34-
3534
/**
3635
* Parameters.
3736
*

‎classes/external/get_file_tree.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
3333
*/
3434
class get_file_tree extends external_api {
35-
3635
/** @var array File extensions considered binary */
3736
private const BINARY_EXTENSIONS = [
3837
'png', 'jpg', 'jpeg', 'gif', 'bmp', 'ico', 'svg', 'webp',

‎classes/external/update_file.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@
3131
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
3232
*/
3333
class update_file extends external_api {
34-
3534
/**
3635
* Parameters.
3736
*

0 commit comments

Comments
 (0)