Add simulation gallery entries and computer links #1160
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -806,6 +806,25 @@ if (MFC_DOCUMENTATION) | |||||||||||||||||||||||
| GEN_DOCS(post_process "MFC: Post-Process") | ||||||||||||||||||||||||
| GEN_DOCS(documentation "MFC") | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Inject per-page last-updated dates into documentation markdown files. | ||||||||||||||||||||||||
| # Runs after auto-generated .md files exist, before Doxygen processes them. | ||||||||||||||||||||||||
| # Uses a stamp file so it only runs once per build. | ||||||||||||||||||||||||
| add_custom_command( | ||||||||||||||||||||||||
| OUTPUT "${CMAKE_CURRENT_BINARY_DIR}/inject-dates.stamp" | ||||||||||||||||||||||||
| DEPENDS "${CMAKE_CURRENT_SOURCE_DIR}/docs/documentation/examples.md" | ||||||||||||||||||||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/docs/documentation/case_constraints.md" | ||||||||||||||||||||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/docs/documentation/physics_constraints.md" | ||||||||||||||||||||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/docs/documentation/cli-reference.md" | ||||||||||||||||||||||||
| "${CMAKE_CURRENT_SOURCE_DIR}/docs/documentation/parameters.md" | ||||||||||||||||||||||||
|
Comment on lines
+814
to
+818
Contributor
There was a problem hiding this comment. P2: The inject-dates custom command runs docs/inject-dates.py but doesn’t list that script in DEPENDS, so updates to the script won’t retrigger the stamp and documentation can go stale without a clean build. Prompt for AI agents
Suggested change
|
||||||||||||||||||||||||
| COMMAND "${Python3_EXECUTABLE}" "${CMAKE_CURRENT_SOURCE_DIR}/docs/inject-dates.py" | ||||||||||||||||||||||||
| "${CMAKE_CURRENT_SOURCE_DIR}" | ||||||||||||||||||||||||
| COMMAND "${CMAKE_COMMAND}" -E touch "${CMAKE_CURRENT_BINARY_DIR}/inject-dates.stamp" | ||||||||||||||||||||||||
| COMMENT "Injecting page dates into documentation" | ||||||||||||||||||||||||
| VERBATIM | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Suggestion: Add VERBATIM to the new custom command so that the Python invocation and paths are safely quoted across platforms, matching the other custom commands in this file. [custom_rule] Severity Level: Minor
Suggested change
Why it matters? ⭐Adding VERBATIM matches the pattern used elsewhere in this CMakeLists (other add_custom_command invocations include VERBATIM) and ensures CMake does not perform platform-specific tokenization/escaping on the Python command and its arguments. This improves robustness/portability of the custom command and directly addresses a portability/consistency concern in the diff. The change is syntactically valid CMake and fixes a real potential issue rather than being purely cosmetic. Prompt for AI Agent 🤖This is a comment left during a code review.
**Path:** CMakeLists.txt
**Line:** 823:823
**Comment:**
*Custom Rule: Add VERBATIM to the new custom command so that the Python invocation and paths are safely quoted across platforms, matching the other custom commands in this file.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. |
||||||||||||||||||||||||
| add_custom_target(inject_page_dates DEPENDS "${CMAKE_CURRENT_BINARY_DIR}/inject-dates.stamp") | ||||||||||||||||||||||||
| add_dependencies(documentation_doxygen inject_page_dates) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # > Copy Resources (main landing page & assets) | ||||||||||||||||||||||||
| install(DIRECTORY "${CMAKE_CURRENT_SOURCE_DIR}/docs/res" | ||||||||||||||||||||||||
| DESTINATION "docs/mfc") | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,20 +23,54 @@ | |||||||||||||||
| <meta property="og:url" content="https://mflowcode.github.io" /> | ||||||||||||||||
| <meta property="og:type" content="website" /> | ||||||||||||||||
| <meta name="twitter:card" content="summary_large_image" /> | ||||||||||||||||
| <script type="application/ld+json"> | ||||||||||||||||
| { | ||||||||||||||||
| "@context": "https://schema.org", | ||||||||||||||||
| "@type": "SoftwareSourceCode", | ||||||||||||||||
| "name": "MFC", | ||||||||||||||||
| "alternateName": "Multi-Component Flow Code", | ||||||||||||||||
| "description": "Open-source exascale multiphase flow solver. 2025 Gordon Bell Prize Finalist. Scales to 200+ trillion grid points on 43,000+ GPUs.", | ||||||||||||||||
| "url": "https://mflowcode.github.io", | ||||||||||||||||
| "license": "https://opensource.org/licenses/MIT", | ||||||||||||||||
| "programmingLanguage": ["Fortran", "Python"], | ||||||||||||||||
| "codeRepository": "https://github.com/MFlowCode/MFC", | ||||||||||||||||
| "author": { | ||||||||||||||||
| "@type": "Organization", | ||||||||||||||||
| "name": "MFlowCode", | ||||||||||||||||
| "url": "https://github.com/MFlowCode" | ||||||||||||||||
| }, | ||||||||||||||||
| "award": "2025 ACM Gordon Bell Prize Finalist" | ||||||||||||||||
| } | ||||||||||||||||
| </script> | ||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||
| <script src="https://cdn.tailwindcss.com"></script> | ||||||||||||||||
| <link rel="stylesheet" href="https://cdnjs.cloudflare.com/ajax/libs/font-awesome/6.4.0/css/all.min.css"> | ||||||||||||||||
| <link rel="icon" type="image/x-icon" href="res/icon.ico"> | ||||||||||||||||
| <script> | ||||||||||||||||
| const sims = [ | ||||||||||||||||
| // Rockets & jets | ||||||||||||||||
| { name: "Mach 12 Starship Super Heavy", image: "res/simulations/o.png", computer: "Alps", computerUrl: "https://www.cscs.ch/computers/alps/", accelerators: "5K GH200s", walltime: "18h", source: "https://www.youtube.com/watch?v=NSn3OVF8N4I" }, | ||||||||||||||||
| { name: "Mach 10 triple jet booster", image: "res/simulations/n.png", computer: "Frontier", computerUrl: "https://www.olcf.ornl.gov/frontier/", accelerators: "10K GCDs", walltime: "12h", source: "https://www.youtube.com/watch?v=pMUl55xqGgM" }, | ||||||||||||||||
| { name: "Mach 2 flow over a sphere", image: "res/simulations/i.png", computer: "Phoenix", computerUrl: "https://www.pace.gatech.edu/", accelerators: "36 V100s", walltime: "30m", source: "https://www.youtube.com/watch?v=HQGSUvYEGqM" }, | ||||||||||||||||
| { name: "Mach 2 shear layer", image: "res/simulations/j.png", computer: "Phoenix", computerUrl: "https://www.pace.gatech.edu/", accelerators: "32 V100s", walltime: "15m", source: "https://www.youtube.com/watch?v=GtcdCHLmJO8" }, | ||||||||||||||||
| // Aerodynamics | ||||||||||||||||
| { name: "Flow over an airfoil (vorticity)", image: "res/simulations/g.png", computer: "Delta", computerUrl: "https://www.ncsa.illinois.edu/research/project-highlights/delta/", accelerators: "128 A100s", walltime: "19h", source: "https://www.youtube.com/watch?v=FvAgnBW59cY" }, | ||||||||||||||||
| { name: "Pitching airfoil (3D)", image: "res/simulations/m.png", computer: "Phoenix", computerUrl: "https://www.pace.gatech.edu/", accelerators: "1 A100", walltime: "5h", source: "https://www.youtube.com/watch?v=2XH-9MumDHU" }, | ||||||||||||||||
| // Shock-droplet | ||||||||||||||||
| { name: "Shedding water droplet", image: "res/simulations/a.png", computer: "Summit", computerUrl: "https://www.olcf.ornl.gov/summit/", accelerators: "960 V100s", walltime: "4h", source: "https://www.youtube.com/watch?v=Gjj-qZkXcrg" }, | ||||||||||||||||
| // Biomedical & acoustics | ||||||||||||||||
| { name: "Burstwave lithotripsy", image: "res/simulations/k.png", computer: "Delta", computerUrl: "https://www.ncsa.illinois.edu/research/project-highlights/delta/", accelerators: "128 A100s", walltime: "30m", source: "https://www.youtube.com/watch?v=XWsUTaJXGF8" }, | ||||||||||||||||
| { name: "Cavitation fragments kidney stone", image: "res/simulations/d.png", computer: "Summit", computerUrl: "https://www.olcf.ornl.gov/summit/", accelerators: "576 V100s", walltime: "30m", source: "https://doi.org/10.48550/arXiv.2305.09163" }, | ||||||||||||||||
| { name: "Kidney stone stress waves", image: "res/simulations/l.png", computer: "Bridges2", computerUrl: "https://www.psc.edu/resources/bridges-2/", accelerators: "8 V100s", walltime: "20m", source: "https://www.youtube.com/watch?v=Q2L0J68qnRw" }, | ||||||||||||||||
| { name: "Whale bubble net feeding", image: "res/simulations/p.png", computer: "Delta", computerUrl: "https://www.ncsa.illinois.edu/research/project-highlights/delta/", accelerators: "128 A100s", walltime: "30m", source: "https://www.youtube.com/watch?v=6EpP6tdCZSA" }, | ||||||||||||||||
| { name: "Earplug acoustics (kinetic energy)", image: "res/simulations/q.png", computer: "Delta", computerUrl: "https://www.ncsa.illinois.edu/research/project-highlights/delta/", accelerators: "8 A100s", walltime: "5h", source: "https://www.youtube.com/watch?v=xSW5wZkdbrc" }, | ||||||||||||||||
| { name: "Circular orifice (1 kHz)", image: "res/simulations/r.png", computer: "Delta", computerUrl: "https://www.ncsa.illinois.edu/research/project-highlights/delta/", accelerators: "16 A100s", walltime: "5h", source: "https://www.youtube.com/watch?v=jOhJ_c7eco4" }, | ||||||||||||||||
| // Bubble dynamics | ||||||||||||||||
| { name: "Collapsing bubbles (pressure)", image: "res/simulations/b.png", computer: "Summit", computerUrl: "https://www.olcf.ornl.gov/summit/", accelerators: "216 V100s", walltime: "3h", source: "https://doi.org/10.48550/arXiv.2305.09163" }, | ||||||||||||||||
| { name: "Collapsing bubbles (streamlines)", image: "res/simulations/c.png", computer: "Summit", computerUrl: "https://www.olcf.ornl.gov/summit/", accelerators: "216 V100s", walltime: "3h", source: "https://doi.org/10.48550/arXiv.2305.09163" }, | ||||||||||||||||
| { name: "Euler-Lagrange particle cloud", image: "res/simulations/s.png", computer: "Phoenix", computerUrl: "https://www.pace.gatech.edu/", accelerators: "8 A100s", walltime: "<1h", source: "https://www.youtube.com/watch?v=RoT-yC5Lxmg" }, | ||||||||||||||||
| // Fundamentals | ||||||||||||||||
| { name: "Breakup of vibrated interface", image: "res/simulations/f.png", computer: "Summit", computerUrl: "https://www.olcf.ornl.gov/summit/", accelerators: "128 V100s", walltime: "4h", source: "https://www.youtube.com/watch?v=XQ3g1oSg8mc" }, | ||||||||||||||||
| { name: "Viscous Taylor-Green vortex", image: "res/simulations/h.png", computer: "Delta", computerUrl: "https://www.ncsa.illinois.edu/research/project-highlights/delta/", accelerators: "128 A100s", walltime: "17h", source: "https://www.youtube.com/watch?v=7i2h08dlDQw" }, | ||||||||||||||||
| ]; | ||||||||||||||||
|
|
||||||||||||||||
| const scalings = [ | ||||||||||||||||
|
|
@@ -54,14 +88,14 @@ | |||||||||||||||
| <img class="h-10" src="res/logo.png" alt=""> | ||||||||||||||||
| </div> | ||||||||||||||||
| <div class="flex-1 p-2 font-semibold text-center">${s.name}</div> | ||||||||||||||||
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank"> | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. P2: Add rel="noopener noreferrer" to the external link opened with target="_blank" to prevent window.opener security risks. Prompt for AI agents
Suggested change
Contributor
There was a problem hiding this comment. Suggestion: Using target="_blank" on external links without rel="noopener noreferrer" allows the opened page to access window.opener and potentially redirect the original page (reverse tabnabbing), so the link to simulation sources should include rel attributes to prevent this. [security] Severity Level: Major
|
||||||||||||||||
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank"> | |
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank" rel="noopener noreferrer"> |
Steps of Reproduction ✅
1. Open `docs/index.html` in a browser (homepage entry point).
2. On DOMContentLoaded, script block at `docs/index.html:82-99` populates `#ft-sim` with
cards, rendering anchor at `docs/index.html:93` as `<a ... href="${s.source}"
target="_blank">` without `rel`.
3. Click any simulation source link (e.g., a YouTube or DOI URL) so it opens in a new tab
due to `target="_blank"`.
4. In the DevTools console of the newly opened tab, execute `window.opener.location =
'https://example.com'` and observe the original `docs/index.html` tab is navigated away,
demonstrating the reverse‑tabnabbing risk caused by missing `rel="noopener noreferrer"`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/index.html
**Line:** 93:93
**Comment:**
*Security: Using target="_blank" on external links without rel="noopener noreferrer" allows the opened page to access window.opener and potentially redirect the original page (reverse tabnabbing), so the link to simulation sources should include rel attributes to prevent this.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Contributor
There was a problem hiding this comment.
Suggestion: Add a rel attribute to the external link opened in a new tab to prevent the new page from accessing the opener context. [custom_rule]
Severity Level: Minor
Suggested change
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank"> | |
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank" rel="noopener noreferrer"> |
Why it matters? ⭐
Adding rel="noopener noreferrer" mitigates reverse tabnabbing and prevents the opened page from accessing window.opener. This is a security best-practice for links using target="_blank" and directly improves the code's safety without changing behavior for legitimate destinations.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/index.html
**Line:** 93:93
**Comment:**
*Custom Rule: Add a rel attribute to the external link opened in a new tab to prevent the new page from accessing the opener context.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Comment on lines
+91
to
+92
Contributor
There was a problem hiding this comment.
Add rel="noopener noreferrer" to all target="_blank" links
Both the source icon link (line 93) and the computer URL link (line 100) open a new tab without rel="noopener noreferrer". This leaves window.opener accessible in the opened tab and leaks the Referer header. These are straightforward to fix:
🔒 Proposed fix
- <a class="w-10 text-center text-xl" href="${s.source}" target="_blank">
+ <a class="w-10 text-center text-xl" href="${s.source}" target="_blank" rel="noopener noreferrer">- <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank">${s.computer}</a>` : s.computer}</div>
+ <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank" rel="noopener noreferrer">${s.computer}</a>` : s.computer}</div>Also applies to: 100-100
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/index.html` around lines 93 - 94, The anchor element rendering the
source icon (the <a class="w-10 text-center text-xl" href="${s.source}"
target="_blank"> element) and any other anchors that open new tabs should
include rel="noopener noreferrer"; update the JSX/HTML that generates this link
(and the computer URL link) to add rel="noopener noreferrer" whenever
target="_blank" is used so window.opener is not exposed and the referrer is not
leaked.
Contributor
There was a problem hiding this comment.
Suggestion: Guard the use of the includes method on the source URL to avoid runtime errors if the field is ever missing or null. [custom_rule]
Severity Level: Minor
Suggested change
| <i class="${s.source.includes('youtube.com') ? 'fa-brands fa-youtube' : 'fa-solid fa-arrow-up-right-from-square'}"></i> | |
| <i class="${s.source && s.source.includes('youtube.com') ? 'fa-brands fa-youtube' : 'fa-solid fa-arrow-up-right-from-square'}"></i> |
Why it matters? ⭐
The proposed change (s.source && s.source.includes(...)) guards against a potential runtime TypeError if a future sims entry omitted source or set it to null/undefined. This is a correctness improvement (runtime-safety) rather than cosmetic, and the improved code is a small, syntactically valid defensive check that directly resolves that issue.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/index.html
**Line:** 94:94
**Comment:**
*Custom Rule: Guard the use of the `includes` method on the source URL to avoid runtime errors if the field is ever missing or null.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Comment on lines
+91
to
93
Contributor
There was a problem hiding this comment.
Source-link icon has no accessible label — screen readers cannot identify the link.
The <a> contains only a FontAwesome <i> icon with no visible text, aria-label, or title. Screen-reader users will hear an unlabelled link.
♿ Proposed fix
- <a class="w-10 text-center text-xl" href="${s.source}" target="_blank">
- <i class="${s.source.includes('youtube.com') ? 'fa-brands fa-youtube' : 'fa-solid fa-arrow-up-right-from-square'}"></i>
+ <a class="w-10 text-center text-xl" href="${s.source}" target="_blank" rel="noopener noreferrer"
+ aria-label="${s.source.includes('youtube.com') ? 'Watch on YouTube' : 'View source'}">
+ <i class="${s.source.includes('youtube.com') ? 'fa-brands fa-youtube' : 'fa-solid fa-arrow-up-right-from-square'}" aria-hidden="true"></i>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank"> | |
| <i class="${s.source.includes('youtube.com') ? 'fa-brands fa-youtube' : 'fa-solid fa-arrow-up-right-from-square'}"></i> | |
| </a> | |
| <a class="w-10 text-center text-xl" href="${s.source}" target="_blank" rel="noopener noreferrer" | |
| aria-label="${s.source.includes('youtube.com') ? 'Watch on YouTube' : 'View source'}"> | |
| <i class="${s.source.includes('youtube.com') ? 'fa-brands fa-youtube' : 'fa-solid fa-arrow-up-right-from-square'}" aria-hidden="true"></i> | |
| </a> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/index.html` around lines 93 - 95, The anchor that renders the source
icon (the <a> that uses s.source and contains the <i> element) lacks an
accessible label; update its markup to include an accessible name by adding an
aria-label (and optionally a title or visually-hidden text) that describes the
destination (e.g., "Open source on YouTube" when s.source includes
'youtube.com', otherwise "Open source link"); ensure the label text is generated
from s.source or context so screen readers receive a meaningful description and
keep the existing icon for visual users.
Contributor
There was a problem hiding this comment.
P2: Add rel="noopener noreferrer" to the computer link opened with target="_blank" to avoid window.opener vulnerabilities.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At docs/index.html, line 100:
<comment>Add rel="noopener noreferrer" to the computer link opened with target="_blank" to avoid window.opener vulnerabilities.</comment>
<file context>
@@ -54,14 +90,14 @@
<div class="flex flex-row items-center">
<div class="pr-2"><i class="fa-solid fa-server"></i></div>
- <div class="flex-1 text-center">${s.computer}</div>
+ <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank">${s.computer}</a>` : s.computer}</div>
</div>
<div class="flex flex-row items-center">
</file context>
Suggested change
| <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank">${s.computer}</a>` : s.computer}</div> | |
| <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank" rel="noopener noreferrer">${s.computer}</a>` : s.computer}</div> |
Contributor
There was a problem hiding this comment.
Suggestion: The dynamically generated external links to HPC center pages also use target="_blank" without rel="noopener noreferrer", which can be abused by the new tab to manipulate the originating page, so the anchor should include rel attributes. [security]
Severity Level: Major ⚠️
- CRITICAL Docs homepage HPC links expose window.opener reference.
- WARN External HPC pages can navigate original documentation tab.
- WARN Security posture of external-resource links is weakened.
Suggested change
| <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank">${s.computer}</a>` : s.computer}</div> | |
| <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank" rel="noopener noreferrer">${s.computer}</a>` : s.computer}</div> |
Steps of Reproduction ✅
1. Open `docs/index.html` in a browser (homepage).
2. DOMContentLoaded handler at `docs/index.html:82-107` renders the server row, producing
the template at `docs/index.html:100` with `<a href="${s.computerUrl}" ...
target="_blank">` and no `rel`.
3. Click any HPC center link (e.g., CSCS Alps, OLCF Frontier) so it opens in a new tab due
to `target="_blank"`.
4. In the DevTools console of that new HPC tab, run `window.opener.location =
'https://example.com'` and see the original docs tab navigated, confirming the
reverse‑tabnabbing condition from missing `rel="noopener noreferrer"`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/index.html
**Line:** 100:100
**Comment:**
*Security: The dynamically generated external links to HPC center pages also use target="_blank" without rel="noopener noreferrer", which can be abused by the new tab to manipulate the originating page, so the anchor should include rel attributes.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Contributor
There was a problem hiding this comment.
Suggestion: Ensure that dynamically generated external computer links opened in a new tab also include a rel attribute for security. [custom_rule]
Severity Level: Minor
Suggested change
| <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank">${s.computer}</a>` : s.computer}</div> | |
| <div class="flex-1 text-center">${s.computerUrl ? `<a href="${s.computerUrl}" class="underline hover:text-amber-400" target="_blank" rel="noopener noreferrer">${s.computer}</a>` : s.computer}</div> |
Why it matters? ⭐
Same as above: adding rel="noopener noreferrer" to programmatically-generated external links prevents window.opener access and improves security. The improved code is a minimal, correct fix that addresses a real security concern and is applicable to this PR hunk.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** docs/index.html
**Line:** 100:100
**Comment:**
*Custom Rule: Ensure that dynamically generated external computer links opened in a new tab also include a rel attribute for security.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,31 @@ | ||||||
| #!/usr/bin/env python3 | ||||||
| """Inject last-updated dates into docs before Doxygen runs. | ||||||
|
|
||||||
| Usage: python3 inject-dates.py [source_dir] | ||||||
| source_dir defaults to current directory. | ||||||
| """ | ||||||
| import subprocess | ||||||
| import sys | ||||||
| from pathlib import Path | ||||||
| from datetime import date | ||||||
|
|
||||||
| src_dir = Path(sys.argv[1]) if len(sys.argv) > 1 else Path(".") | ||||||
| docs_dir = src_dir / "docs" / "documentation" | ||||||
|
|
||||||
| for md_file in sorted(docs_dir.glob("*.md")): | ||||||
| if "Page last updated:" in md_file.read_text(): | ||||||
| continue | ||||||
|
|
||||||
| result = subprocess.run( | ||||||
| ["git", "log", "-1", "--format=%as", "--", str(md_file)], | ||||||
| capture_output=True, text=True, | ||||||
| cwd=str(src_dir), | ||||||
| ) | ||||||
| page_date = result.stdout.strip() or str(date.today()) | ||||||
|
|
||||||
| with open(md_file, "a") as f: | ||||||
|
Contributor
There was a problem hiding this comment. Missing Without an explicit encoding, the file is opened using the platform default, which is not 🐛 Proposed fix- with open(md_file, "a") as f:
+ with open(md_file, "a", encoding="utf-8") as f:📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Contributor
There was a problem hiding this comment. P2: Add Prompt for AI agents |
||||||
| f.write( | ||||||
| f"\n\n<div style='text-align:center; font-size:0.75rem; " | ||||||
| f"color:#888; padding:16px 0 0;'>" | ||||||
| f"Page last updated: {page_date}</div>\n" | ||||||
| ) | ||||||
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| User-agent: * | ||
| Allow: / | ||
|
|
||
| Sitemap: https://mflowcode.github.io/sitemap.xml |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Lychee now runs after publish — broken links won't block deployment on scheduled/dispatch runs.
Moving Lychee after the Publish step (with
fail: true) means that onschedule/workflow_dispatchtriggers, documentation is deployed to GitHub Pages before link checking occurs. A link-check failure will mark the workflow as failed but won't roll back the published content.On PRs this is not an issue (the publish step is skipped), so Lychee correctly gates PR merges. For production runs, this trade-off appears intentional per the PR description ("move publish step before lychee"), but it's worth confirming that alerting-on-failure (e.g., via GitHub notification or branch protection) is sufficient to catch post-deploy regressions.
🤖 Prompt for AI Agents