From 952d14a83a79ed8f99674cdea19cea43f64cdaf8 Mon Sep 17 00:00:00 2001
From: abhinav-phi <alpha9coder@gmail.com>
Date: Thu, 17 Sep 2026 15:58:24 +0530
Subject: [PATCH 1/3] Modernize module iframes while preserving dashboard flow
 (#914)

Replace deprecated frameborder string injection with DOM-created sandboxed frames. Adds title, loading lazy, referrerpolicy same-origin, and same-origin sandbox that blocks top navigation but keeps scripts, forms, modals, popups and downloads for training content. Adds same-origin src validation and fixes getStarted double-html handling. No Java or session logic changed.
---
 src/main/webapp/css/theCss.css |  1 +
 src/main/webapp/getStarted.jsp | 21 +++++++---
 src/main/webapp/index.jsp      | 76 ++++++++++++++++++++++++++++++----
 3 files changed, 84 insertions(+), 14 deletions(-)

diff --git a/src/main/webapp/css/theCss.css b/src/main/webapp/css/theCss.css
index 748ce84e0..ff4d66355 100644
--- a/src/main/webapp/css/theCss.css
+++ b/src/main/webapp/css/theCss.css
@@ -390,6 +390,7 @@ a:hover {
 	word-wrap: break-word;
 	width: 100%;
 	height: 2056px;
+	border: 0;
 }
 
 .levelList {
diff --git a/src/main/webapp/getStarted.jsp b/src/main/webapp/getStarted.jsp
index 579f4fb6d..c2ef4e98a 100644
--- a/src/main/webapp/getStarted.jsp
+++ b/src/main/webapp/getStarted.jsp
@@ -199,12 +199,23 @@ else
 	var theRefreshError = "Could not Refresh Menu";
 
 	$('#getStarted').slideDown("slow");
-	$('#cantSee').html("<iframe class='levelIframe' frameborder='no' id='theStart' src='readyToPlay.jsp'></iframe>");
-	$('#cantSee').html(function(){
-		$("#theStart").load(function(){
-			$("#contentDiv").slideDown("slow");
-		});
+	// Issue 914: keep same-origin sandboxed frame for readyToPlay welcome view.
+	// readyToPlay is a full document with its own session check, so it stays
+	// isolated in the frame while dashboard navigation and session handling
+	// stay outside. Replaces deprecated frameborder string injection.
+	$("#cantSee").empty();
+	var startFrame = document.createElement("iframe");
+	startFrame.setAttribute("id", "theStart");
+	startFrame.setAttribute("class", "levelIframe");
+	startFrame.setAttribute("title", "Security Shepherd getting started");
+	startFrame.setAttribute("loading", "lazy");
+	startFrame.setAttribute("referrerpolicy", "same-origin");
+	startFrame.setAttribute("sandbox", "allow-same-origin allow-scripts allow-forms allow-modals allow-popups allow-popups-to-escape-sandbox allow-downloads");
+	startFrame.setAttribute("src", "readyToPlay.jsp");
+	$(startFrame).on("load", function(){
+		$("#contentDiv").slideDown("slow");
 	});
+	$("#cantSee").append(startFrame);
 	<% if (isAdmin) { %>
 	$("#allApplication").click(function(){
         if ($('#unsafeLevels').is(":checked"))
diff --git a/src/main/webapp/index.jsp b/src/main/webapp/index.jsp
index f751bc971..8aa064831 100644
--- a/src/main/webapp/index.jsp
+++ b/src/main/webapp/index.jsp
@@ -770,6 +770,43 @@ if (request.getSession() != null)
 	%>
 
 	<script>
+			// Issue 914 bounded iframe handling: lesson and challenge views are full
+			// documents with their own scripts and relative paths, while result
+			// submission stays in this dashboard. Keep a same-origin sandboxed frame
+			// so navigation, authentication, session handling and lesson behaviour
+			// are preserved. Top-level navigation stays blocked so untrusted module
+			// content cannot break out of the dashboard. Popups, modals, forms and
+			// downloads stay allowed because existing lessons use target blank
+			// external links, alert based training flows and file references.
+			var SHEPHERD_FRAME_SANDBOX = "allow-same-origin allow-scripts allow-forms allow-modals allow-popups allow-popups-to-escape-sandbox allow-downloads";
+			function isSafeModuleSrc(src) {
+				if (!src || typeof src !== "string") {
+					return false;
+				}
+				var trimmed = src.replace(/^\s+|\s+$/g, "");
+				if (trimmed === "") {
+					return false;
+				}
+				var lower = trimmed.toLowerCase();
+				if (lower.indexOf("javascript:") === 0 || lower.indexOf("data:") === 0 || lower.indexOf("vbscript:") === 0) {
+					return false;
+				}
+				if (trimmed.indexOf("://") !== -1 || trimmed.indexOf("//") === 0) {
+					return false;
+				}
+				return true;
+			}
+			function createModuleFrame(frameId, src, title) {
+				var frame = document.createElement("iframe");
+				frame.setAttribute("id", frameId);
+				frame.setAttribute("class", "levelIframe");
+				frame.setAttribute("title", title);
+				frame.setAttribute("loading", "lazy");
+				frame.setAttribute("referrerpolicy", "same-origin");
+				frame.setAttribute("sandbox", SHEPHERD_FRAME_SANDBOX);
+				frame.setAttribute("src", src);
+				return frame;
+			}
 			function applyMenuButtonActionsOpenOrTourney(theCsrfToken, theErrorMessage){
 				console.log("Applying Menu Actions For Open/Tourney");
 				
@@ -791,8 +828,13 @@ if (request.getSession() != null)
 						if(ajaxCall.status == 200)
 						{
 							theActualFile = ajaxCall.responseText;
-							$('#contentDiv').html("<iframe frameborder='no' class='levelIframe' id='theChallenge' src='" + theActualFile + "'></iframe>");
-							$("#theChallenge").load(function(){
+							if (!isSafeModuleSrc(theActualFile)) {
+								$('#contentDiv').html("<p> <fmt:message key="generic.text.sorryError" />: invalid module address</p>");
+								$("#contentDiv").slideDown("slow");
+							} else {
+							$("#contentDiv").empty();
+							var challengeFrame = createModuleFrame("theChallenge", theActualFile, "Security Shepherd challenge module");
+							$(challengeFrame).on("load", function(){
 								<%if (showCheatSheet) {%>
 									$("#submitResult").slideDown("fast", function(){
 										$("#cheatSheetButton").slideDown("fast", function(){
@@ -818,8 +860,10 @@ if (request.getSession() != null)
 										});
 									});
 								<%}%>
-							}).appendTo('#contentDiv');
+							});
+							$("#contentDiv").append(challengeFrame);
 							$("#theSidebarWrapper").height($("#contentDiv").height());
+							}
 						}
 						else
 						{
@@ -847,8 +891,13 @@ if (request.getSession() != null)
 						if(ajaxCall.status == 200)
 						{
 							theActualFile = ajaxCall.responseText;
-							$('#contentDiv').html("<iframe frameborder='no' class='levelIframe' id='theLesson' src='" + theActualFile + "'></iframe>");
-							$("#theLesson").load(function(){
+							if (!isSafeModuleSrc(theActualFile)) {
+								$('#contentDiv').html("<p> <fmt:message key="generic.text.sorryError" />: invalid module address</p>");
+								$("#contentDiv").slideDown("slow");
+							} else {
+							$("#contentDiv").empty();
+							var lessonFrame = createModuleFrame("theLesson", theActualFile, "Security Shepherd lesson module");
+							$(lessonFrame).on("load", function(){
 							<%if (showCheatSheet) {%>
 								$("#submitResult").slideDown("fast", function(){
 									$("#cheatSheetButton").slideDown("fast", function(){
@@ -874,8 +923,10 @@ if (request.getSession() != null)
 									});
 								});
 							<%}%>
-							}).appendTo('#contentDiv');
+							});
+							$("#contentDiv").append(lessonFrame);
 							$("#theSidebarWrapper").height($("#contentDiv").height());
+							}
 						}
 						else
 						{
@@ -918,8 +969,13 @@ if (request.getSession() != null)
 						});
 						if(ajaxCall.status == 200) {
 							theActualFile = ajaxCall.responseText;
-							$('#contentDiv').html("<iframe frameborder='no' class='levelIframe' id='theLesson' src='" + theActualFile + "'></iframe>");
-							$("#theLesson").load(function(){
+							if (!isSafeModuleSrc(theActualFile)) {
+								$('#contentDiv').html("<p> " + theErrorMessage + ": invalid module address</p>");
+								$("#contentDiv").slideDown("slow");
+							} else {
+							$("#contentDiv").empty();
+							var ctfLessonFrame = createModuleFrame("theLesson", theActualFile, "Security Shepherd lesson module");
+							$(ctfLessonFrame).on("load", function(){
 								<%if (showCheatSheet) {%>
 									$("#submitResult").slideDown("fast", function(){
 										$("#cheatSheetButton").slideDown("fast", function(){
@@ -945,7 +1001,9 @@ if (request.getSession() != null)
 										});
 									});
 								<%}%>
-							}).appendTo('#contentDiv');
+							});
+							$("#contentDiv").append(ctfLessonFrame);
+							}
 						} else {
 							$('#contentDiv').html("<p> " + theErrorMessage + ": " + ajaxCall.status + " " + ajaxCall.statusText + "</p>");
 							$("#contentDiv").slideDown("slow");

From 79680c10d0bc90cc5b0cfe2f901706081b0db119 Mon Sep 17 00:00:00 2001
From: Abhinav <alpha9coder@gmail.com>
Date: Sun, 11 Oct 2026 05:10:30 +0530
Subject: [PATCH 2/3] fix(modernize): drop lazy loading on hidden module
 iframes

SeanDuggan tested this branch locally and found that every challenge,
lesson and the welcome view stayed blank: the frames live inside a div
that starts hidden, and lazy loading never triggers for a hidden element,
so the iframe never fires load and the UI that waits on it never reveals.

Removed the loading attribute from createModuleFrame in index.jsp and from
the readyToPlay frame in getStarted.jsp, with a note at each site so this
is not re-added as a performance win. The rest of the modernization is
unchanged: same-origin sandbox, referrerpolicy, explicit titles and the
class based sizing that replaces the deprecated frameborder injection.
---
 src/main/webapp/getStarted.jsp | 3 ++-
 src/main/webapp/index.jsp      | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/src/main/webapp/getStarted.jsp b/src/main/webapp/getStarted.jsp
index c2ef4e98a..58455cc25 100644
--- a/src/main/webapp/getStarted.jsp
+++ b/src/main/webapp/getStarted.jsp
@@ -208,7 +208,8 @@ else
 	startFrame.setAttribute("id", "theStart");
 	startFrame.setAttribute("class", "levelIframe");
 	startFrame.setAttribute("title", "Security Shepherd getting started");
-	startFrame.setAttribute("loading", "lazy");
+	// no loading="lazy" here: the frame sits in a hidden div until it fires load
+	// itself, so lazy loading never triggers and the welcome view stays blank
 	startFrame.setAttribute("referrerpolicy", "same-origin");
 	startFrame.setAttribute("sandbox", "allow-same-origin allow-scripts allow-forms allow-modals allow-popups allow-popups-to-escape-sandbox allow-downloads");
 	startFrame.setAttribute("src", "readyToPlay.jsp");
diff --git a/src/main/webapp/index.jsp b/src/main/webapp/index.jsp
index 8aa064831..9c76a2122 100644
--- a/src/main/webapp/index.jsp
+++ b/src/main/webapp/index.jsp
@@ -801,7 +801,8 @@ if (request.getSession() != null)
 				frame.setAttribute("id", frameId);
 				frame.setAttribute("class", "levelIframe");
 				frame.setAttribute("title", title);
-				frame.setAttribute("loading", "lazy");
+				// no loading="lazy" here: these frames start hidden and lazy loading never
+				// triggers for a hidden element, so the lesson never loads (issue 914 review)
 				frame.setAttribute("referrerpolicy", "same-origin");
 				frame.setAttribute("sandbox", SHEPHERD_FRAME_SANDBOX);
 				frame.setAttribute("src", src);

From 393fd6b351a9300f66eff309ef5a6e0f83c66442 Mon Sep 17 00:00:00 2001
From: SeanDuggan <SeanDuggan@users.noreply.github.com>
Date: Sat, 17 Oct 2026 18:30:45 +0100
Subject: [PATCH 3/3] Trim PR #920 explanatory comments; localize
 module-address error

- Remove the two multi-line "Issue 914" rationale comments from
  getStarted.jsp and index.jsp; that context belongs in the PR
  description/commit message, not inline, and would rot as the
  code changes.
- Add generic.text.invalidModuleAddress message key and use
  fmt:message for the "invalid module address" error text in
  index.jsp instead of hardcoded English, consistent with every
  other user-facing string in the file.
---
 src/main/resources/i18n/text.properties |  1 +
 src/main/webapp/getStarted.jsp          |  4 ----
 src/main/webapp/index.jsp               | 14 +++-----------
 3 files changed, 4 insertions(+), 15 deletions(-)

diff --git a/src/main/resources/i18n/text.properties b/src/main/resources/i18n/text.properties
index c7993ee31..ead07c756 100644
--- a/src/main/resources/i18n/text.properties
+++ b/src/main/resources/i18n/text.properties
@@ -24,6 +24,7 @@ generic.text.scoreboard = Scoreboard
 generic.text.searchModules = Search Modules
 generic.text.shepherdVersion = Security Shepherd Version: 3.0
 generic.text.sorryError = Sorry but there was an error
+generic.text.invalidModuleAddress = invalid module address
 generic.text.submitResult = Submit Result Key Here
 generic.text.submit = Submit
 generic.text.username = Username
diff --git a/src/main/webapp/getStarted.jsp b/src/main/webapp/getStarted.jsp
index 58455cc25..bd5b80889 100644
--- a/src/main/webapp/getStarted.jsp
+++ b/src/main/webapp/getStarted.jsp
@@ -199,10 +199,6 @@ else
 	var theRefreshError = "Could not Refresh Menu";
 
 	$('#getStarted').slideDown("slow");
-	// Issue 914: keep same-origin sandboxed frame for readyToPlay welcome view.
-	// readyToPlay is a full document with its own session check, so it stays
-	// isolated in the frame while dashboard navigation and session handling
-	// stay outside. Replaces deprecated frameborder string injection.
 	$("#cantSee").empty();
 	var startFrame = document.createElement("iframe");
 	startFrame.setAttribute("id", "theStart");
diff --git a/src/main/webapp/index.jsp b/src/main/webapp/index.jsp
index 9c76a2122..fff4ad4f2 100644
--- a/src/main/webapp/index.jsp
+++ b/src/main/webapp/index.jsp
@@ -770,14 +770,6 @@ if (request.getSession() != null)
 	%>
 
 	<script>
-			// Issue 914 bounded iframe handling: lesson and challenge views are full
-			// documents with their own scripts and relative paths, while result
-			// submission stays in this dashboard. Keep a same-origin sandboxed frame
-			// so navigation, authentication, session handling and lesson behaviour
-			// are preserved. Top-level navigation stays blocked so untrusted module
-			// content cannot break out of the dashboard. Popups, modals, forms and
-			// downloads stay allowed because existing lessons use target blank
-			// external links, alert based training flows and file references.
 			var SHEPHERD_FRAME_SANDBOX = "allow-same-origin allow-scripts allow-forms allow-modals allow-popups allow-popups-to-escape-sandbox allow-downloads";
 			function isSafeModuleSrc(src) {
 				if (!src || typeof src !== "string") {
@@ -830,7 +822,7 @@ if (request.getSession() != null)
 						{
 							theActualFile = ajaxCall.responseText;
 							if (!isSafeModuleSrc(theActualFile)) {
-								$('#contentDiv').html("<p> <fmt:message key="generic.text.sorryError" />: invalid module address</p>");
+								$('#contentDiv').html("<p> <fmt:message key="generic.text.sorryError" />: <fmt:message key="generic.text.invalidModuleAddress" /></p>");
 								$("#contentDiv").slideDown("slow");
 							} else {
 							$("#contentDiv").empty();
@@ -893,7 +885,7 @@ if (request.getSession() != null)
 						{
 							theActualFile = ajaxCall.responseText;
 							if (!isSafeModuleSrc(theActualFile)) {
-								$('#contentDiv').html("<p> <fmt:message key="generic.text.sorryError" />: invalid module address</p>");
+								$('#contentDiv').html("<p> <fmt:message key="generic.text.sorryError" />: <fmt:message key="generic.text.invalidModuleAddress" /></p>");
 								$("#contentDiv").slideDown("slow");
 							} else {
 							$("#contentDiv").empty();
@@ -971,7 +963,7 @@ if (request.getSession() != null)
 						if(ajaxCall.status == 200) {
 							theActualFile = ajaxCall.responseText;
 							if (!isSafeModuleSrc(theActualFile)) {
-								$('#contentDiv').html("<p> " + theErrorMessage + ": invalid module address</p>");
+								$('#contentDiv').html("<p> " + theErrorMessage + ": <fmt:message key="generic.text.invalidModuleAddress" /></p>");
 								$("#contentDiv").slideDown("slow");
 							} else {
 							$("#contentDiv").empty();