Conversation
✅ Deploy Preview for cozystack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughThe changes introduce an OSS Health feature displaying project metrics (DevStats, OpenSSF Best Practices, OSS Insight) for Cozystack. This includes a Python script that fetches data from external sources, new Hugo content pages and layout templates for rendering, SCSS styling, a GitHub Actions workflow for monthly automated updates, and corresponding JSON data files. Changes
Sequence Diagram(s)sequenceDiagram
participant GHA as GitHub Actions
participant Script as update_oss_health.py
participant GH as GitHub API
participant DevStats as DevStats Service
participant OSI as OSS Insight
participant OpenSSF as OpenSSF
participant Repo as Git Repository
participant PR as Pull Request
GHA->>Script: Execute (monthly/manual)
Script->>GH: Fetch repo metadata & contributors
Script->>DevStats: Fetch dev statistics
Script->>OSI: Fetch repository insights
Script->>OpenSSF: Parse best practices status
Script->>Script: Aggregate metrics (month/quarter/year)
Script->>Repo: Write JSON to data/ & static/
GHA->>Repo: Check for staged changes
alt Changes detected
GHA->>Repo: Create/update update-oss-health branch
GHA->>Repo: Commit with timestamp
GHA->>Repo: Force-push branch
GHA->>PR: Create PR to main (if not exists)
end
sequenceDiagram
participant User as User Browser
participant Hugo as Hugo Site
participant Template as oss-health-app.html
participant JS as Client JavaScript
participant API as /oss-health-data/ JSON
User->>Hugo: Request /oss-health/devstats/
Hugo->>Template: Render page
Template->>User: Return HTML with data-attributes
User->>JS: JavaScript initializes
JS->>API: Fetch ${key}.json
API->>JS: Return metrics JSON
alt oss_health_kind = "timeseries"
JS->>JS: Render period tabs (month/quarter/year)
JS->>JS: Render summary cards
JS->>JS: Render contributor tables
JS->>JS: Render language cloud
else oss_health_kind = "state"
JS->>JS: Render state badge
JS->>JS: Render status details
JS->>JS: Render action buttons
end
JS->>User: Display OSS Health dashboard
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request introduces an OSS Health dashboard to the website, featuring automated snapshots of project activity and security posture. It includes a new Python script to fetch data from GitHub, DevStats, and OpenSSF, along with the necessary Hugo layouts, SCSS styling, and content pages to display these metrics. The review feedback highlights several improvements for the data collection script, specifically suggesting the use of the OpenSSF JSON API instead of fragile HTML scraping, correcting a locale-specific URL, and broadening exception handling to ensure the fallback logic for contributor stats is more robust against network errors.
| GITHUB_API = "https://api.github.com" | ||
| OPENSSF_PROJECT_URL = "https://www.bestpractices.dev/projects/10177" | ||
| OPENSSF_BADGE_URL = "https://www.bestpractices.dev/projects/10177/badge" | ||
| OPENSSF_STATUS_URL = "https://www.bestpractices.dev/pt-BR/projects/10177/passing" |
There was a problem hiding this comment.
The OPENSSF_STATUS_URL is currently pointing to a Portuguese (pt-BR) version of the page. This might lead to issues if the scraping logic (regex) expects English text or if the site structure differs across locales. It is better to use the generic or English URL, or ideally, the JSON API provided by OpenSSF.
| OPENSSF_STATUS_URL = "https://www.bestpractices.dev/pt-BR/projects/10177/passing" | |
| OPENSSF_STATUS_URL = "https://www.bestpractices.dev/en/projects/10177/passing" |
| def parse_openssf_state(page_text: str) -> str: | ||
| lowered = page_text.lower() | ||
| if "passing" in lowered: | ||
| return "Passing" | ||
| if "in progress" in lowered: | ||
| return "In Progress" | ||
| return "Unknown" | ||
|
|
||
|
|
||
| def parse_openssf_last_updated(page_text: str) -> str | None: | ||
| match = re.search(r"last updated on\s+(\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2} UTC)", page_text, re.IGNORECASE) | ||
| if not match: | ||
| return None | ||
| try: | ||
| dt = datetime.strptime(match.group(1), "%Y-%m-%d %H:%M:%S UTC").replace(tzinfo=UTC) | ||
| except ValueError: | ||
| return None | ||
| return isoformat(dt) | ||
|
|
||
|
|
||
| def parse_openssf_project_name(page_text: str) -> str | None: | ||
| match = re.search(r"<title>\s*([^<]+?)\s*\|\s*BadgeApp", page_text, re.IGNORECASE) | ||
| if not match: | ||
| return None | ||
| return unescape(match.group(1).strip()) |
There was a problem hiding this comment.
| try: | ||
| contributor_stats = github_contributor_stats(headers) | ||
| except RuntimeError: |
There was a problem hiding this comment.
The current fallback logic only catches RuntimeError. However, github_contributor_stats performs network requests using urllib.request.urlopen, which can raise urllib.error.URLError or urllib.error.HTTPError (e.g., on rate limits or API downtime). These exceptions should also be caught to ensure the fallback to raw commits works as intended.
| try: | |
| contributor_stats = github_contributor_stats(headers) | |
| except RuntimeError: | |
| try: | |
| contributor_stats = github_contributor_stats(headers) | |
| except (RuntimeError, urllib.error.URLError): |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
assets/scss/main.scss (1)
159-159: Stylelint flags invalid@importposition.The
@import "oss_health"statement appears after non-import rules (body,astyles at lines 133-146). Per CSS spec,@importrules should appear before other rules. However, this follows the existing pattern in the file where other custom imports (lines 149-161) also appear after these rules, so this may be intentional or an existing technical debt.Consider verifying the import order works correctly in your SCSS compilation pipeline.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/scss/main.scss` at line 159, The `@import` "oss_health" statement is placed after top-level rules (e.g., the body and a selectors), which triggers Stylelint because `@import` rules must appear before other rules; move the `@import` "oss_health" line (and any other custom imports currently after those selectors) to the top of the stylesheet before the body/a rule blocks, or if the build pipeline intentionally requires imports later, add a comment and update the Stylelint config to allow this pattern; refer to the `@import` "oss_health" token and the body/a selector blocks to locate the change.data/oss-health/devstats.json (1)
1-496: Data file duplication betweendata/andstatic/directories can be removed.The
data/oss-health/directory duplicatesstatic/oss-health-data/. The update script intentionally writes to both directories, but the client-side renderer only fetches from/oss-health-data/(static). Thedata/copy is not used by any Hugo templates. Consider removing thedata/oss-health/duplication to simplify maintenance.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/oss-health/devstats.json` around lines 1 - 496, Remove the unused duplicate dataset in data/oss-health (e.g., data/oss-health/devstats.json) and stop writing to it from the update script; instead keep a single canonical copy under static/oss-health-data/ (used by the client at /oss-health-data/). Edit the update script that currently writes both locations so it writes only to static/oss-health-data/, and verify Hugo templates do not reference data/oss-health/ (search for data/oss-health or devstats.json) before deleting the data/oss-health directory.assets/scss/_oss_health.scss (1)
92-99: Respect reduced-motion user preference for the pulse animation.The infinite pulse animation should be disabled for users who prefer reduced motion.
♿ Proposed refinement
.oss-health-shell { @@ &__pulse-dot { animation: oss-health-pulse 1.8s ease-in-out infinite; @@ } } + +@media (prefers-reduced-motion: reduce) { + .oss-health-shell__pulse-dot { + animation: none; + } +} `@keyframes` oss-health-pulse {Also applies to: 408-418
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@assets/scss/_oss_health.scss` around lines 92 - 99, The pulse animation on the BEM element &__pulse-dot (animation: oss-health-pulse 1.8s ease-in-out infinite) must respect prefers-reduced-motion: wrap a media query `@media` (prefers-reduced-motion: reduce) that targets the same selector and disable the animation (set animation: none or animation-play-state: paused) and/or remove infinite repetition so users who prefer reduced motion won't see the pulsing; apply the same change to the other occurrences of the pulse selector (lines referenced 408-418).layouts/_default/oss-health-app.html (1)
170-173: Use button-group semantics (or fully implement ARIA tabs) for the period switcher.Current
tablist/tabroles are partial. If you keep tabs, add full tab behavior; otherwise a simple grouped toggle (role="group"+aria-pressed) is more accurate.♿ Proposed refinement
- <div class="oss-health-switcher" role="tablist" aria-label="Report period"> + <div class="oss-health-switcher" role="group" aria-label="Report period"> ${order.map((name) => ` - <button class="oss-health-switcher__button${name === active ? " is-active" : ""}" data-period="${name}" role="tab" aria-selected="${name === active ? "true" : "false"}"> + <button type="button" class="oss-health-switcher__button${name === active ? " is-active" : ""}" data-period="${name}" aria-pressed="${name === active ? "true" : "false"}"> ${escapeHtml(payload.periods[name].label)} </button> `).join("")} </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@layouts/_default/oss-health-app.html` around lines 170 - 173, The period switcher using the oss-health-switcher container and oss-health-switcher__button elements currently exposes partial tab semantics (role="tablist"/role="tab")—either implement full ARIA Tabs behavior for keyboard and focus management in the component (activateTab, onKeyDown handlers, aria-controls, tabindex management) or simplify the markup to a toggle button group: change role="tablist" to role="group" on the oss-health-switcher container, remove role="tab" from oss-health-switcher__button and instead add aria-pressed="{name === active}", ensure each button has type="button" and update any JS that reads data-period to use aria-pressed for state; this keeps semantics correct without a full tab implementation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/update-oss-health.yaml:
- Around line 1-10: The workflow "Update OSS health snapshots" performs git push
and gh pr create but lacks explicit GITHUB_TOKEN permissions; add a top-level
permissions block for the workflow (not per-job) that grants at least contents:
write and pull-requests: write so the push and PR creation succeed in
environments with read-only default tokens, e.g., insert a permissions section
with keys "contents" set to "write" and "pull-requests" set to "write" near the
top of the workflow (alongside name/on) to enable the write operations executed
in the update-oss-health job.
In `@hack/update_oss_health.py`:
- Line 209: The current code builds pulls by filtering all_pulls on created_at
(pulls = [pull for pull in all_pulls if since <=
parse_datetime(pull["created_at"]) <= until]) which misses PRs opened before
since but merged inside the window; change to compute a separate merged_pulls
list by filtering on merged_at (e.g. merged_pulls = [p for p in all_pulls if
p.get("merged_at") and since <= parse_datetime(p["merged_at"]) <= until]) and
use merged_pulls to compute merged_prs (and any merged-PR metrics) instead of
reusing pulls; update any logic referencing merged_prs (and the blocks around
the current pulls/merged computation) to use merged_pulls so merge counts
reflect merge time rather than creation time.
- Around line 219-223: The current loop counts entire GitHub "weeks" blocks
(using contributor.get("weeks", []") and adds week["c"] when week_end > since,
which inflates totals for partial-week overlaps; change the logic to count
individual commits within the exact date window instead of using weekly
aggregates: replace the weekly-sum code that updates commits_in_period with code
that iterates the contributor's raw commit timestamps (or fetches individual
commit dates from the API) and increments commits_in_period only when
commit_date >= since and commit_date < until (use the existing variable names
week_start/week_end/since/until/commits_in_period to find the spot). Ensure any
API call to retrieve per-commit timestamps is done once per contributor and
handle timezone-aware datetime comparison as currently used.
- Line 29: The OPENSSF_STATUS_URL is hardcoded to a Portuguese path which will
break when state changes and causes the English-only regex for
badge_last_updated_at to fail; update the code that defines OPENSSF_STATUS_URL
to point to the project base URL (no locale/state suffix) and modify
parse_openssf_state() to build the full status page URL from the detected state
(e.g., using the state string to append '/{state}' when fetching), and adjust
the badge_last_updated_at extraction to accept localized variants or derive the
timestamp from a locale-agnostic element (e.g., a datetime attribute or a
standardized HTML element) so parsing works regardless of page language.
In `@layouts/_default/oss-health-app.html`:
- Around line 71-75: The renderLink helper currently uses escapeHtml(item.url)
but does not validate URL schemes, allowing unsafe schemes to be injected;
update renderLink to sanitize and validate the URL before inserting into href by
parsing item.url and permitting only safe schemes (e.g., http, https, optionally
mailto), and if the URL is invalid or uses a disallowed scheme, return the
escaped text without an href (or use a safe fallback like '#'); apply the same
URL validation/sanitization logic to the other link/image render sites
referenced (the occurrences around lines 214-217 and 232-234) so all href/src
values are checked before being embedded.
In `@layouts/partials/footer.html`:
- Around line 7-12: The DOM order of the footer columns is left → right → center
but visually on sm+ it's left → center → right; swap the two div blocks so the
center column's element (class td-footer__center rendering partial
"footer/center.html") appears before the right column's element (class
td-footer__right rendering partial "footer/right.html") in the markup to align
keyboard/screen-reader focus order with the visual order while keeping the
existing order-sm classes intact.
---
Nitpick comments:
In `@assets/scss/_oss_health.scss`:
- Around line 92-99: The pulse animation on the BEM element &__pulse-dot
(animation: oss-health-pulse 1.8s ease-in-out infinite) must respect
prefers-reduced-motion: wrap a media query `@media` (prefers-reduced-motion:
reduce) that targets the same selector and disable the animation (set animation:
none or animation-play-state: paused) and/or remove infinite repetition so users
who prefer reduced motion won't see the pulsing; apply the same change to the
other occurrences of the pulse selector (lines referenced 408-418).
In `@assets/scss/main.scss`:
- Line 159: The `@import` "oss_health" statement is placed after top-level rules
(e.g., the body and a selectors), which triggers Stylelint because `@import` rules
must appear before other rules; move the `@import` "oss_health" line (and any
other custom imports currently after those selectors) to the top of the
stylesheet before the body/a rule blocks, or if the build pipeline intentionally
requires imports later, add a comment and update the Stylelint config to allow
this pattern; refer to the `@import` "oss_health" token and the body/a selector
blocks to locate the change.
In `@data/oss-health/devstats.json`:
- Around line 1-496: Remove the unused duplicate dataset in data/oss-health
(e.g., data/oss-health/devstats.json) and stop writing to it from the update
script; instead keep a single canonical copy under static/oss-health-data/ (used
by the client at /oss-health-data/). Edit the update script that currently
writes both locations so it writes only to static/oss-health-data/, and verify
Hugo templates do not reference data/oss-health/ (search for data/oss-health or
devstats.json) before deleting the data/oss-health directory.
In `@layouts/_default/oss-health-app.html`:
- Around line 170-173: The period switcher using the oss-health-switcher
container and oss-health-switcher__button elements currently exposes partial tab
semantics (role="tablist"/role="tab")—either implement full ARIA Tabs behavior
for keyboard and focus management in the component (activateTab, onKeyDown
handlers, aria-controls, tabindex management) or simplify the markup to a toggle
button group: change role="tablist" to role="group" on the oss-health-switcher
container, remove role="tab" from oss-health-switcher__button and instead add
aria-pressed="{name === active}", ensure each button has type="button" and
update any JS that reads data-period to use aria-pressed for state; this keeps
semantics correct without a full tab implementation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: adc7ca12-5304-4f09-a591-16ce1c952bab
📒 Files selected for processing (21)
.github/workflows/update-oss-health.yamlMakefileassets/scss/_oss_health.scssassets/scss/main.scsscontent/en/oss-health/_index.mdcontent/en/oss-health/devstats.mdcontent/en/oss-health/openssf.mdcontent/en/oss-health/oss-insight.mddata/oss-health/devstats.jsondata/oss-health/openssf.jsondata/oss-health/ossinsight.jsondata/oss-health/summary.jsonhack/update_oss_health.pyhugo.yamllayouts/_default/oss-health-app.htmllayouts/partials/footer.htmllayouts/partials/footer/center.htmlstatic/oss-health-data/devstats.jsonstatic/oss-health-data/openssf.jsonstatic/oss-health-data/ossinsight.jsonstatic/oss-health-data/summary.json
| name: Update OSS health snapshots | ||
|
|
||
| on: | ||
| workflow_dispatch: | ||
| schedule: | ||
| - cron: '0 4 1 * *' | ||
|
|
||
| jobs: | ||
| update-oss-health: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify missing explicit permissions and presence of write operations
rg -n '^\s*permissions:' .github/workflows/update-oss-health.yaml || true
rg -n 'git push|gh pr create' .github/workflows/update-oss-health.yamlRepository: cozystack/website
Length of output: 160
🏁 Script executed:
cat -n .github/workflows/update-oss-health.yamlRepository: cozystack/website
Length of output: 2341
Add explicit workflow token permissions for push and PR creation operations.
The workflow performs write operations (git push at line 42 and gh pr create at lines 54–58) without declaring explicit permissions. In environments where the default GITHUB_TOKEN is read-only, these operations will fail. Add the following permissions block:
🔧 Proposed fix
name: Update OSS health snapshots
on:
workflow_dispatch:
schedule:
- cron: '0 4 1 * *'
+permissions:
+ contents: write
+ pull-requests: write
+
jobs:
update-oss-health:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/update-oss-health.yaml around lines 1 - 10, The workflow
"Update OSS health snapshots" performs git push and gh pr create but lacks
explicit GITHUB_TOKEN permissions; add a top-level permissions block for the
workflow (not per-job) that grants at least contents: write and pull-requests:
write so the push and PR creation succeed in environments with read-only default
tokens, e.g., insert a permissions section with keys "contents" set to "write"
and "pull-requests" set to "write" near the top of the workflow (alongside
name/on) to enable the write operations executed in the update-oss-health job.
| GITHUB_API = "https://api.github.com" | ||
| OPENSSF_PROJECT_URL = "https://www.bestpractices.dev/projects/10177" | ||
| OPENSSF_BADGE_URL = "https://www.bestpractices.dev/projects/10177/badge" | ||
| OPENSSF_STATUS_URL = "https://www.bestpractices.dev/pt-BR/projects/10177/passing" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Configured URLs:"
sed -n '27,31p' hack/update_oss_health.py
echo
python - <<'PY'
import json
for path in ("data/oss-health/openssf.json", "static/oss-health-data/openssf.json"):
with open(path, encoding="utf-8") as f:
payload = json.load(f)
print(path, {k: payload.get(k) for k in ("status_url", "state", "badge_last_updated_at")})
PY
for url in \
'https://www.bestpractices.dev/projects/10177' \
'https://www.bestpractices.dev/pt-BR/projects/10177/passing'
do
echo
echo "Markers from $url"
curl -sSL "$url" \
| rg -n -i 'last updated|atualizad|passing|in progress|BadgeApp' \
| sed -n '1,20p'
doneRepository: cozystack/website
Length of output: 5348
🏁 Script executed:
cat -n hack/update_oss_health.py | head -100Repository: cozystack/website
Length of output: 4094
🏁 Script executed:
sed -n '350,400p' hack/update_oss_health.pyRepository: cozystack/website
Length of output: 1810
🏁 Script executed:
grep -n "badge_last_updated_at\|last updated\|last_updated" hack/update_oss_health.pyRepository: cozystack/website
Length of output: 318
Hardcoded status URL prevents following state changes and incompatible with Portuguese page fetch.
The URL hardcodes /pt-BR/...passing, but parse_openssf_state() (lines 365–372) dynamically detects state from the page. If the project status changes to "In Progress", the URL won't match the new state path. Additionally, the regex for extracting badge_last_updated_at (line 360) searches for the English phrase "last updated on", which doesn't exist on the Portuguese page—this explains why the field is null in both snapshot files. Fetching a localized page while parsing with an English-only regex is fundamentally broken.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/update_oss_health.py` at line 29, The OPENSSF_STATUS_URL is hardcoded to
a Portuguese path which will break when state changes and causes the
English-only regex for badge_last_updated_at to fail; update the code that
defines OPENSSF_STATUS_URL to point to the project base URL (no locale/state
suffix) and modify parse_openssf_state() to build the full status page URL from
the detected state (e.g., using the state string to append '/{state}' when
fetching), and adjust the badge_last_updated_at extraction to accept localized
variants or derive the timestamp from a locale-agnostic element (e.g., a
datetime attribute or a standardized HTML element) so parsing works regardless
of page language.
| all_pulls: list[dict], | ||
| all_commits: list[dict] | None = None, | ||
| ) -> dict: | ||
| pulls = [pull for pull in all_pulls if since <= parse_datetime(pull["created_at"]) <= until] |
There was a problem hiding this comment.
PRs Merged is currently limited to PRs opened in the same window.
pulls has already been filtered by created_at, so a PR opened before since and merged inside the period never increments merged_prs. That underreports the merge metric in both the DevStats and OSS Insight summary cards. This needs a merge-time query or dataset instead of reusing the opened-PR list.
Also applies to: 239-247
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/update_oss_health.py` at line 209, The current code builds pulls by
filtering all_pulls on created_at (pulls = [pull for pull in all_pulls if since
<= parse_datetime(pull["created_at"]) <= until]) which misses PRs opened before
since but merged inside the window; change to compute a separate merged_pulls
list by filtering on merged_at (e.g. merged_pulls = [p for p in all_pulls if
p.get("merged_at") and since <= parse_datetime(p["merged_at"]) <= until]) and
use merged_pulls to compute merged_prs (and any merged-PR metrics) instead of
reusing pulls; update any logic referencing merged_prs (and the blocks around
the current pulls/merged computation) to use merged_pulls so merge counts
reflect merge time rather than creation time.
| for week in contributor.get("weeks", []): | ||
| week_start = datetime.fromtimestamp(int(week["w"]), tz=UTC) | ||
| week_end = week_start + timedelta(days=7) | ||
| if week_end > since and week_start <= until: | ||
| commits_in_period += int(week.get("c", 0)) |
There was a problem hiding this comment.
This counts whole weeks even when only part of the week is in range.
Any contributor week overlapping since is added in full, so the "Last 30/90/365 days" cards can include commits from before the requested window. If these pages are meant to show exact day ranges, the contributor totals need to come from raw commits rather than GitHub's weekly aggregates.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hack/update_oss_health.py` around lines 219 - 223, The current loop counts
entire GitHub "weeks" blocks (using contributor.get("weeks", []") and adds
week["c"] when week_end > since, which inflates totals for partial-week
overlaps; change the logic to count individual commits within the exact date
window instead of using weekly aggregates: replace the weekly-sum code that
updates commits_in_period with code that iterates the contributor's raw commit
timestamps (or fetches individual commit dates from the API) and increments
commits_in_period only when commit_date >= since and commit_date < until (use
the existing variable names week_start/week_end/since/until/commits_in_period to
find the spot). Ensure any API call to retrieve per-commit timestamps is done
once per contributor and handle timezone-aware datetime comparison as currently
used.
| const renderLink = (item) => { | ||
| const text = escapeHtml(item.name); | ||
| if (!item.url) return text; | ||
| return `<a href="${escapeHtml(item.url)}" target="_blank" rel="noopener noreferrer">${text}</a>`; | ||
| }; |
There was a problem hiding this comment.
Sanitize outbound URLs before injecting into href/src.
escapeHtml() is not URL validation. A payload URL with an unsafe scheme could still be injected into links/images.
🛡️ Proposed fix
const escapeHtml = (value) => String(value ?? "")
@@
.replaceAll('"', """)
.replaceAll("'", "'");
+
+ const sanitizeUrl = (value) => {
+ if (!value) return "";
+ try {
+ const parsed = new URL(value, window.location.origin);
+ if (parsed.protocol === "http:" || parsed.protocol === "https:") {
+ return parsed.href;
+ }
+ } catch (_) {}
+ return "";
+ };
const renderLink = (item) => {
const text = escapeHtml(item.name);
- if (!item.url) return text;
- return `<a href="${escapeHtml(item.url)}" target="_blank" rel="noopener noreferrer">${text}</a>`;
+ const href = sanitizeUrl(item.url);
+ if (!href) return text;
+ return `<a href="${escapeHtml(href)}" target="_blank" rel="noopener noreferrer">${text}</a>`;
};
@@
const renderOpenSSF = (payload) => {
+ const projectUrl = sanitizeUrl(payload.project_url);
+ const badgeUrl = sanitizeUrl(payload.badge_url);
+ const statusUrl = sanitizeUrl(payload.status_url);
@@
- <a href="${escapeHtml(payload.project_url)}" target="_blank" rel="noopener noreferrer">
- <img src="${escapeHtml(payload.badge_url)}" alt="OpenSSF Best Practices badge for Cozystack">
+ <a href="${escapeHtml(projectUrl)}" target="_blank" rel="noopener noreferrer">
+ <img src="${escapeHtml(badgeUrl)}" alt="OpenSSF Best Practices badge for Cozystack">
</a>
@@
- <a class="btn btn-primary" href="${escapeHtml(payload.project_url)}" target="_blank" rel="noopener noreferrer">Open project page</a>
- <a class="btn btn-outline-primary" href="${escapeHtml(payload.status_url)}" target="_blank" rel="noopener noreferrer">Canonical status page</a>
+ <a class="btn btn-primary" href="${escapeHtml(projectUrl)}" target="_blank" rel="noopener noreferrer">Open project page</a>
+ <a class="btn btn-outline-primary" href="${escapeHtml(statusUrl)}" target="_blank" rel="noopener noreferrer">Canonical status page</a>
</div>Also applies to: 214-217, 232-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@layouts/_default/oss-health-app.html` around lines 71 - 75, The renderLink
helper currently uses escapeHtml(item.url) but does not validate URL schemes,
allowing unsafe schemes to be injected; update renderLink to sanitize and
validate the URL before inserting into href by parsing item.url and permitting
only safe schemes (e.g., http, https, optionally mailto), and if the URL is
invalid or uses a disallowed scheme, return the escaped text without an href (or
use a safe fallback like '#'); apply the same URL validation/sanitization logic
to the other link/image render sites referenced (the occurrences around lines
214-217 and 232-234) so all href/src values are checked before being embedded.
| <div class="td-footer__right col-6 col-sm-4 order-sm-3"> | ||
| {{ partial "footer/right.html" . }} | ||
| </div> | ||
| <div class="td-footer__center col-12 col-sm-4 py-2 order-sm-2"> | ||
| {{ partial "footer/center.html" . }} | ||
| </div> |
There was a problem hiding this comment.
Keep the footer's source order aligned with the visual order.
On sm+, the CSS order utilities display the center column between left and right, but keyboard and screen-reader navigation still follows the DOM order here: left → right → center. Swapping the center/right blocks keeps focus order consistent with what users see.
♿ Proposed fix
- <div class="td-footer__right col-6 col-sm-4 order-sm-3">
- {{ partial "footer/right.html" . }}
- </div>
<div class="td-footer__center col-12 col-sm-4 py-2 order-sm-2">
{{ partial "footer/center.html" . }}
</div>
+ <div class="td-footer__right col-6 col-sm-4 order-sm-3">
+ {{ partial "footer/right.html" . }}
+ </div>📝 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.
| <div class="td-footer__right col-6 col-sm-4 order-sm-3"> | |
| {{ partial "footer/right.html" . }} | |
| </div> | |
| <div class="td-footer__center col-12 col-sm-4 py-2 order-sm-2"> | |
| {{ partial "footer/center.html" . }} | |
| </div> | |
| <div class="td-footer__center col-12 col-sm-4 py-2 order-sm-2"> | |
| {{ partial "footer/center.html" . }} | |
| </div> | |
| <div class="td-footer__right col-6 col-sm-4 order-sm-3"> | |
| {{ partial "footer/right.html" . }} | |
| </div> |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@layouts/partials/footer.html` around lines 7 - 12, The DOM order of the
footer columns is left → right → center but visually on sm+ it's left → center →
right; swap the two div blocks so the center column's element (class
td-footer__center rendering partial "footer/center.html") appears before the
right column's element (class td-footer__right rendering partial
"footer/right.html") in the markup to align keyboard/screen-reader focus order
with the visual order while keeping the existing order-sm classes intact.
Summary
Verification
Summary by CodeRabbit
New Features
Documentation
Chores