Conversation
…pt for index generation - Deleted index.php and moved its functionality to a new Node.js script (scripts/generate-index.js). - Introduced package.json and package-lock.json for managing dependencies and scripts. - Implemented logic to read publication domains and their metadata from pubDomainList.json. - Created a structured HTML output for documentation based on publication domains and document types. - Removed utils.php as its functions are no longer needed in the new structure.
- Added "serve" as a dependency to facilitate serving the application. - Introduced a "start" script to run the application on a specified port.
|
|
||
| if: github.ref == 'refs/heads/main' || github.ref == 'refs/heads/develop' | ||
| env: | ||
| AZURE_CONTAINER_REGISTRY: geonovumregistrytest.azurecr.io |
There was a problem hiding this comment.
test registry? fixen voor merge gok ik
En wellicht gewoon 'CONTAINER_REGISTRY' noemen en uit de global settings halen ipv per repos setten.
| <h3><a href="./bhr-g">Booronderzoek (BHR-G)</a></h3> | ||
| <h3><a href="./BHR-GT">Booronderzoek: Geotechnische boormonsterbeschrijving en boormonsteranalyse (BHR-GT)</a></h3> |
There was a problem hiding this comment.
absolute paden hebben vanuit security perspectief de voorkeur. Al maakt het in dit geval weinig uit. De . voor de slash voegt echter ook weinig toe.
| @@ -0,0 +1,29 @@ | |||
| <!DOCTYPE html> | |||
There was a problem hiding this comment.
Kan dit niet in de htaccess als redirect?
De JS in deze pagina is valide, maar maakt het lastiger om met een Nonce te gaan werken in de context van CSP headers.
| { key: 'documentatie', heading: 'Documentatie' }, | ||
| ]; | ||
|
|
||
| const TEMPLATE_START = `<!DOCTYPE html> |
There was a problem hiding this comment.
idealiter maken we 1 template file, los op disk, met knappe placeholders.
| <meta name="viewport" content="width=device-width, initial-scale=1, shrink-to-fit=no"> | ||
| <title>Geonovum specificaties</title> | ||
| <link rel='shortcut icon' type='image/x-icon' href='https://tools.geostandaarden.nl/respec/style/logos/Geonovum.ico' /> | ||
| <style> |
There was a problem hiding this comment.
Stylesheet naar een losse file, (danwel gebruiken van geonovum hosted stylesteet). Inline, danwel infile css maakt de CSP header minder strikt. Ook zijn losse css files te chachen.
| try { | ||
| const html = await buildIndexPage(); | ||
| await fs.writeFile(OUTPUT_FILE, html); | ||
| console.log(`Updated ${path.relative(rootDir, OUTPUT_FILE)}`); |
There was a problem hiding this comment.
Iets van sanitization op logregels is altijd verstandig.
https://snyk.io/blog/prevent-log-injection-vulnerability-javascript-node-js/
underdarknl
left a comment
There was a problem hiding this comment.
Een paar ideen om de boel net wat makkelijker te maken in de toekomst.
… update image name setting
There was a problem hiding this comment.
Pull request overview
This PR migrates the site from PHP-driven dynamic pages to a fully static site by generating index.html ahead of time with Node.js, and updates the delivery pipeline to build/push a container image.
Changes:
- Replace PHP index generation/redirect endpoints with static HTML equivalents, plus a Node-based index generator.
- Add Node tooling (
package.json, lockfile) and a Docker build that generates the landing page during image build. - Update GitHub Actions workflow from rsync-based deployment to container build & push.
Reviewed changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| utils.php | Removes PHP filesystem utility helpers (no longer needed for static flow). |
| scripts/generate-index.js | Adds Node script to generate index.html from repo folders + pubDomainList.json. |
| package.json | Adds Node scripts for index generation and static serving via serve. |
| package-lock.json | Locks Node dependencies for reproducible builds. |
| index.php | Removes the previous PHP-driven landing page. |
| index.html | Adds a generated static landing page output. |
| imroi/index.php | Removes PHP redirect endpoint for imroi. |
| imroi/index.html | Adds static HTML redirect page for imroi. |
| githubConfig.php | Removes PHP GitHub webhook config (no longer used). |
| dsgo/index.php | Removes PHP redirect endpoint for dsgo. |
| dsgo/index.html | Adds static HTML redirect page for dsgo. |
| brtnext/index.php | Removes PHP redirect endpoint for brtnext. |
| brtnext/index.html | Adds static HTML redirect page for brtnext. |
| bro/index.html | Removes embedded PHP logic; replaces with static HTML content. |
| bro/gen/cors.php | Removes PHP CORS proxy endpoint. |
| biblio.php | Removes PHP bibliography aggregation endpoint. |
| README.md | Documents how to regenerate index.html using the Node script. |
| Dockerfile | Adds container build that runs npm ci + generates index.html, then serves the static site. |
| .github/workflows/deploy.yml | Switches CI from rsync deploy to building/pushing a container image + scheduled linkcheck. |
| .dockerignore | Adds Docker context exclusions. |
Comments suppressed due to low confidence (3)
githubConfig.php:1
- The
hubSecretvalue is hardcoded in source, which exposes the GitHub webhook secret to anyone who can read the code or logs and makes rotation difficult. An attacker with this secret can forge webhook requests and potentially trigger or tamper with deployments. Move this secret to a secure configuration mechanism (e.g., environment variable or secret store) and remove it from version control.
biblio.php:1 - The
stream_context_createcall disables TLS certificate validation (verify_peerandverify_peer_nameset tofalse), which makes HTTPS connections tohttps://docs.geostandaarden.nlvulnerable to man-in-the-middle attacks and silent content tampering. An attacker on the network path could modify the fetched HTML/JSON so that this script processes and serves attacker-controlled data. Enable full TLS verification in the stream context or rely on the default PHP HTTPS settings instead of turning off certificate checks.
bro/gen/cors.php:1 - The CORS headers set
Access-Control-Allow-Origindirectly from$_SERVER['HTTP_ORIGIN']while also enablingAccess-Control-Allow-Credentials: true, which effectively trusts any requesting origin with access to credentialed responses. If the proxied URL ever returns user-specific or sensitive data, a malicious website can read that data via JavaScript by making cross-origin requests to this endpoint in a victim's browser. Restrict allowed origins to a strict allowlist and avoid combining wildcard-like origin reflection with credentialed requests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Added `generate:serve-config` script to convert .htaccess rules to serve.json for local serving. - Introduced `build` script to generate index and serve configuration. - Created `styles/main.css` for centralized styling. - Implemented template system for index generation using `templates/index.html`. - Enhanced `generate-index.js` to utilize the new template and escape HTML content. - Updated `package.json` to reflect new scripts and dependencies.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 23 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (3)
biblio.php:1
- Here
stream_context_createdisables TLS certificate and hostname verification (verify_peerandverify_peer_nameare set tofalse) for requests tohttps://docs.geostandaarden.nl, effectively accepting any certificate for that host. An on-path attacker could intercept or tamper with the fetched HTML/JSON and cause this script to process and reflect attacker-controlled data despite using HTTPS. Enable proper TLS verification (including hostname checks and a trusted CA bundle) so that remote content is only accepted from authenticated servers.
bro/gen/cors.php:1 - This CORS helper exposes a server-side HTTP proxy where
$_GET['url']controls the full URL passed intofile_get_contents, and the only validation isstrpos($url, 'https://docs.geostandaarden.nl/bro/gen/') > -1, which can be bypassed by embedding that string in the path or query of an arbitrary host. An attacker can abuse this to make the server issue requests to internal or otherwise restricted services (for examplehttp://127.0.0.1/https://docs.geostandaarden.nl/bro/gen/) and read the responses, leading to SSRF and potential data exfiltration; combined with the reflectedAccess-Control-Allow-OriginandAccess-Control-Allow-Credentialsheaders, browser-based attackers can also read these responses cross-origin. Remove this endpoint or strictly whitelist and parse the destination host and protocol instead of using a substring check, and avoid reflectingHTTP_ORIGINwhen enabling CORS.
githubConfig.php:1 - This config file hardcodes the GitHub webhook secret in
$hubSecret, so anyone with read access to the repository can obtain the real shared secret used to authenticate incoming webhooks. If the same value is configured on GitHub, an attacker could forge signed webhook requests to your receiver, potentially triggering unauthorized builds or deployments or exfiltrating build-time secrets. Move this secret into a secure secret store or environment variable that is not committed to source control, and inject it into the runtime configuration instead of hardcoding it in PHP.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>BRTnext doorverwijzing</title> | ||
| <meta http-equiv="refresh" content="0; url=https://docs.geostandaarden.nl/brtnext/vv-bd-brtnext-20230526/" /> |
There was a problem hiding this comment.
Switching from a server-side Location redirect (PHP) to a client-side meta refresh changes the redirect semantics: crawlers/clients won’t see an HTTP 301/302, caching and SEO signals can differ, and non-HTML clients won’t follow it. If you still need an actual HTTP redirect, consider expressing this as a redirect rule in the server/static host config (e.g., serve.json redirects / reverse proxy config) instead of meta refresh.
| <meta http-equiv="refresh" content="0; url=https://docs.geostandaarden.nl/brtnext/vv-bd-brtnext-20230526/" /> |
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>DSGO doorverwijzing</title> | ||
| <meta http-equiv="refresh" content="0; url=https://docs.geostandaarden.nl/dsgo/vv-hr-DSGO-20220316/" /> |
There was a problem hiding this comment.
Switching from a server-side Location redirect (PHP) to a client-side meta refresh changes the redirect semantics: crawlers/clients won’t see an HTTP 301/302, caching and SEO signals can differ, and non-HTML clients won’t follow it. If you still need an actual HTTP redirect, consider expressing this as a redirect rule in the server/static host config (e.g., serve.json redirects / reverse proxy config) instead of meta refresh.
| <meta http-equiv="refresh" content="0; url=https://docs.geostandaarden.nl/dsgo/vv-hr-DSGO-20220316/" /> |
| <head> | ||
| <meta charset="utf-8" /> | ||
| <title>IMROI doorverwijzing</title> | ||
| <meta http-equiv="refresh" content="0; url=https://docs.geostandaarden.nl/imroi/cv-im-imroi-20240521" /> |
There was a problem hiding this comment.
Switching from a server-side Location redirect (PHP) to a client-side meta refresh changes the redirect semantics: crawlers/clients won’t see an HTTP 301/302, caching and SEO signals can differ, and non-HTML clients won’t follow it. If you still need an actual HTTP redirect, consider expressing this as a redirect rule in the server/static host config (e.g., serve.json redirects / reverse proxy config) instead of meta refresh.
| <meta http-equiv="refresh" content="0; url=https://docs.geostandaarden.nl/imroi/cv-im-imroi-20240521" /> |
| const relDir = path.relative(rootDir, path.dirname(htaccessPath)).replace(/\\/g, '/'); | ||
| const source = `/${relDir}`; | ||
|
|
||
| // Redirect both the bare directory and any sub-paths within it | ||
| redirects.push({ source, destination, type: 301 }); | ||
| redirects.push({ source: `${source}/:path*`, destination: `${destination}:path*`, type: 301 }); | ||
| } | ||
|
|
||
| const config = { redirects }; | ||
| await fs.writeFile(OUTPUT_FILE, JSON.stringify(config, null, 2) + '\n'); | ||
| console.log(`Generated serve.json with ${redirects.length / 2} redirect(s)`); |
There was a problem hiding this comment.
source is built as /${relDir}. When relDir is empty (e.g., a .htaccess in the repo root), source becomes / and the generated wildcard redirect becomes //:path* due to ${source}/:path*. Consider normalizing source (special-case root and trim trailing slashes) so you never emit double slashes and optionally add an explicit redirect for the trailing-slash form (/foo/) as well as /foo.
| const relDir = path.relative(rootDir, path.dirname(htaccessPath)).replace(/\\/g, '/'); | |
| const source = `/${relDir}`; | |
| // Redirect both the bare directory and any sub-paths within it | |
| redirects.push({ source, destination, type: 301 }); | |
| redirects.push({ source: `${source}/:path*`, destination: `${destination}:path*`, type: 301 }); | |
| } | |
| const config = { redirects }; | |
| await fs.writeFile(OUTPUT_FILE, JSON.stringify(config, null, 2) + '\n'); | |
| console.log(`Generated serve.json with ${redirects.length / 2} redirect(s)`); | |
| const relDir = path | |
| .relative(rootDir, path.dirname(htaccessPath)) | |
| .replace(/\\/g, '/') | |
| .replace(/\/+$/, ''); | |
| const source = relDir ? `/${relDir}` : ''; | |
| const baseSource = source || '/'; | |
| // Redirect the bare directory path (or root) and any sub-paths within it | |
| redirects.push({ source: baseSource, destination, type: 301 }); | |
| if (source) { | |
| // Also redirect the trailing-slash form for non-root directories (e.g., /foo/) | |
| redirects.push({ source: `${source}/`, destination, type: 301 }); | |
| } | |
| redirects.push({ | |
| source: `${baseSource}/:path*`, | |
| destination: `${destination}:path*`, | |
| type: 301, | |
| }); | |
| } | |
| const config = { redirects }; | |
| await fs.writeFile(OUTPUT_FILE, JSON.stringify(config, null, 2) + '\n'); | |
| console.log(`Generated serve.json with ${redirects.length} redirect(s)`); |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Updated the "start" script in package.json to use a new Node.js script (scripts/start.js) for cross-platform compatibility. - Enhanced the serve configuration generation script (scripts/generate-serve-config.js) to support redirects from both .htaccess RewriteRule directives and <meta http-equiv="refresh"> tags in index.html files. - Improved logging to indicate the number of redirects generated from each source.
…ep in deploy workflow
…andling for linkchecker
No description provided.