chore: Enable manual workflow runs and fix codecov for forks

[davidshq]

Summary

Enables manual workflow runs for the build and test workflow and fixes codecov to only run when the token is configured and the project is opensearch-project/OpenSearch-Dashboards (so forks don't fail on missing codecov token).

• This PR addresses chore: Improve CI Experience for Forks #11487

Changes

• [chore] Make manual runs possible for the build and test workflow
• [chore] Don't run codecov if token isn't configured or project isn't opensearch-project/OpenSearch-Dashboards

Changelog

• skip

[github-actions]

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

[github-actions]

PR Code Analyzer ❗

AI-powered 'Code-Diff-Analyzer' found issues on commit 3fad0bc.

Path	Line	Severity	Description
src/plugins/vis_type_table/public/components/table_vis_dynamic_table.tsx	27	low	DOMPurify hook sets forceKeepAttr=true for all 'target' attribute values, bypassing DOMPurify's default behavior of stripping target attributes. While a follow-up hook adds rel='noopener noreferrer' for non-_self targets, this also preserves target='_top' and target='_parent', which can redirect the top-level frame in framed contexts. Appears intentional to fix the 'open link in new tab' bug (see changelog), not malicious, but loosens the sanitizer's default security posture.
.github/workflows/build_and_test_workflow.yml	12	low	Addition of workflow_dispatch trigger allows any user with repository write access to manually trigger the CI workflow against arbitrary refs. Combined with the Codecov token existence check (which expands the secret into a shell string), this is a low-risk but minor expansion of the workflow's attack surface. No evidence of malicious intent; the pattern is common and the secret value is not echoed.

The table above displays the top 10 most important findings.

Total: 2 | Critical: 0 | High: 0 | Medium: 0 | Low: 2

---

Pull Requests Author(s): Please update your Pull Request according to the report above.

Repository Maintainer(s): You can bypass diff analyzer by adding label skip-diff-analyzer after reviewing the changes carefully, then re-run failed actions. To re-enable the analyzer, remove the label, then re-run all actions.

---

⚠️ Note: The Code-Diff-Analyzer helps protect against potentially harmful code patterns. Please ensure you have thoroughly reviewed the changes beforehand.

Thanks.

[github-actions]

❌ Invalid Changelog Heading

The '## Changelog' heading in your PR description is either missing or malformed. Please make sure that your PR description includes a '## Changelog' heading with proper spelling, capitalization, spacing, and Markdown syntax.

[github-actions]

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "Make manual runs possible for the build and test workflow". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

[github-actions]

PR Reviewer Guide 🔍

(Review updated until commit 0d197bf)

Here are some key observations to aid the review process:

🧪 No relevant tests

🔒 Security concerns Sensitive information exposure: The CODECOV_TOKEN secret is directly interpolated into a shell run step via ${{ secrets.CODECOV_TOKEN }}. While GitHub Actions masks known secret values in logs, interpolating secrets directly into shell scripts is generally discouraged as it can be vulnerable to script injection or unexpected log exposure. It is safer to pass the secret as an environment variable (e.g., env: CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}) and reference it as $CODECOV_TOKEN in the shell script.

✅ No TODO sections

🔀 No multiple PR themes

⚡ Recommended focus areas for review Logic Issue The condition steps.check-codecov.outputs.has_token == 'true' || github.repository == 'opensearch-project/OpenSearch-Dashboards' may be redundant or misleading. If the repository is opensearch-project/OpenSearch-Dashboards, the codecov upload will run regardless of whether the token is set, which could cause the codecov action to fail if the token is missing. Consider using && instead of ||, or ensuring the token check is always required. if: steps.check-codecov.outputs.has_token == 'true' || github.repository == 'opensearch-project/OpenSearch-Dashboards' Secret Exposure Checking secrets.CODECOV_TOKEN directly in a run step by interpolating it with ${{ secrets.CODECOV_TOKEN }} can expose the secret value in workflow logs if GitHub's secret masking fails or if the secret contains special characters. A safer approach is to pass the secret as an environment variable and check the environment variable instead. if [ -n "${{ secrets.CODECOV_TOKEN }}" ]; then echo "has_token=true" >> $GITHUB_OUTPUT else echo "has_token=false" >> $GITHUB_OUTPUT fi

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 0d197bf
Explore these optional code suggestions:

Category	Suggestion	Impact
Security	Avoid exposing secrets in shell scripts Directly referencing secrets.CODECOV_TOKEN in a run step exposes whether the secret is set via the expression being evaluated in the shell script, and for forked PRs the secret will be empty anyway. Instead, use an env variable to pass the secret, which is the recommended approach for using secrets in run steps and avoids potential secret exposure in logs. .github/workflows/build_and_test_workflow.yml [113-118] +env: + CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }} run: | - if [ -n "${{ secrets.CODECOV_TOKEN }}" ]; then + if [ -n "$CODECOV_TOKEN" ]; then echo "has_token=true" >> $GITHUB_OUTPUT else echo "has_token=false" >> $GITHUB_OUTPUT fi Suggestion importance[1-10]: 7 __ Why: Using env to pass secrets to shell scripts is the recommended GitHub Actions practice, as it avoids potential secret exposure in logs and handles forked PRs more safely. The improved code correctly shows how to use an env block at the step level to pass secrets.CODECOV_TOKEN as an environment variable.	Medium

---

Previous suggestions

Suggestions up to commit 3fad0bc

Category	Suggestion	Impact
Possible issue	Fix base64 extraction to handle commas in data The code splits on the first comma to strip the data URI prefix, but if the base64 data itself contains a comma (which is valid in base64 padding contexts or edge cases), split(',')[1] would only return the content up to the next comma. Use indexOf with slice instead to safely extract everything after the first comma. src/plugins/chat/public/utils/read_file_as_base64.ts [33-36] reader.onload = () => { const dataUrl = reader.result as string; // Strip the "data:<mime>;base64," prefix - const base64 = dataUrl.split(',')[1] || ''; + const commaIndex = dataUrl.indexOf(','); + const base64 = commaIndex >= 0 ? dataUrl.slice(commaIndex + 1) : ''; Suggestion importance[1-10]: 7 __ Why: Base64 characters do not include commas (the alphabet is A-Z, a-z, 0-9, +, /, =), so split(',')[1] is actually safe for standard base64. However, using indexOf + slice is still a more robust pattern that correctly handles the data URI prefix regardless, making this a valid improvement for correctness and clarity.	Medium
Security	Restrict allowed target attribute values in DOMPurify hook The uponSanitizeAttribute hook unconditionally preserves any target attribute value, including potentially dangerous ones like target="_parent" or target="_top" which could be used for clickjacking. The hook should only allow safe values like _blank and _self. src/plugins/vis_type_table/public/components/table_vis_dynamic_table.tsx [24-28] dompurify.addHook('uponSanitizeAttribute', (node, event) => { if (event.attrName === 'target') { - event.forceKeepAttr = true; + const safeTargets = ['_blank', '_self', '_parent', '_top']; + if (safeTargets.includes(event.attrValue)) { + event.forceKeepAttr = true; + } } }); Suggestion importance[1-10]: 6 __ Why: The security concern is valid — unconditionally preserving any target value could allow _parent or _top which might be used for clickjacking in framed contexts. However, the afterSanitizeElements hook already adds rel="noopener noreferrer" for non-_self targets, which mitigates the main risk.	Low
General	Account for non-attachment payload overhead in size limit The proxyMaxBytes calculation multiplies maxFileUploadBytes by maxFileAttachments, but this doesn't account for the rest of the request body (conversation history, text messages, metadata). For large histories with many messages, the actual payload could exceed this limit and cause legitimate requests to be rejected with a 413. Consider adding a reasonable fixed overhead (e.g., a few MB) to account for non-attachment content. src/plugins/chat/server/routes/index.ts [206-209] +const FIXED_OVERHEAD_BYTES = 5 * 1024 * 1024; // 5MB for conversation history and metadata const proxyMaxBytes = maxFileUploadBytes !== undefined - ? Math.ceil(maxFileUploadBytes * maxFileAttachments * BASE64_OVERHEAD_FACTOR) + ? Math.ceil(maxFileUploadBytes * maxFileAttachments * BASE64_OVERHEAD_FACTOR) + FIXED_OVERHEAD_BYTES : undefined; Suggestion importance[1-10]: 5 __ Why: The suggestion is valid — the current formula only accounts for attachment bytes and doesn't include conversation history or metadata overhead. However, the impact is moderate since the formula already uses a 1.4x overhead factor, and the comment in the code acknowledges this is an approximation.	Low
	Guard against out-of-bounds index in file removal The handleRemoveFile callback accesses prev[index] which could be undefined if the index is out of bounds (e.g., due to a race condition or stale closure). While .filter(Boolean) handles the undefined case for clearAttachmentBase64, the filter still runs correctly. However, it's safer to guard against the invalid index explicitly to avoid silently doing nothing when an invalid index is passed. src/plugins/chat/public/components/chat_window.tsx [492-497] const handleRemoveFile = useCallback((index: number) => { setFileAttachments((prev) => { - clearAttachmentBase64([prev[index]].filter(Boolean)); + if (index < 0 || index >= prev.length) return prev; + clearAttachmentBase64([prev[index]]); return prev.filter((_, i) => i !== index); }); }, []); Suggestion importance[1-10]: 4 __ Why: The suggestion adds a bounds check that prevents silent failures on invalid indices. The existing .filter(Boolean) already handles the undefined case for clearAttachmentBase64, so this is a minor defensive improvement rather than a critical bug fix.	Low

[github-actions]

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "Make manual runs possible for the build and test workflow". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

[github-actions]

Persistent review updated to latest commit 0d197bf

[github-actions]

❌ Invalid Prefix For Manual Changeset Creation

Invalid description prefix. Found "[chore] Make manual runs possible for the build and test workflow". Only "skip" entry option is permitted for manual commit of changeset files.

If you were trying to skip the changelog entry, please use the "skip" entry option in the ##Changelog section of your PR description.

[davidshq]

I believe the prefix issue is fixed? I updated to skip and I see the label was updated to Skip-Changelog, so this should be ready to run through GitHub workflows.