fix: tighten require_repo_name regex and add issue.user null guard

[lml2468]

Follow-up to #19

Addresses two non-blocking observations from the final approved review:

O1 — require_repo_name() permits . and ..

The previous regex [A-Za-z0-9._-]{1,100} accepted . and .. as valid repo names. While neither is a valid GitHub repo name and would just 404, the whole point of the validation is to make path-traversal impossible by construction.

Fix: Require a leading alphanumeric and explicitly reject . / ..:

if not re.fullmatch(r'[A-Za-z0-9][A-Za-z0-9._-]{0,99}', val) or val in {'.', '..'}:

O5 — issue.user null guard

issue.user is theoretically nullable for ghost/deleted accounts. Added a null check before accessing issue.user.type.

Files changed

• .github/workflows/octo-ci-status.yml
• .github/workflows/octo-issue-feed.yml
• .github/workflows/octo-pr-feed.yml
• .github/workflows/issue-welcome.yml

[yujiawei]

Code Review — PR #22 (.github)

Follow-up to #19 addressing two non-blocking observations (O1, O5). Small, focused, low-risk diff: +5/-4 across 4 workflow files.

Verdict: APPROVED

Both fixes are correct and address the stated observations. One minor edge-case worth noting but not blocking.

1. Verification

Item	Status	Evidence
O1: regex requires leading alphanumeric	✅	octo-ci-status.yml:85, octo-issue-feed.yml:93, octo-pr-feed.yml:110 all updated to r'[A-Za-z0-9][A-Za-z0-9._-]{0,99}'
O1: explicit . / .. rejection	✅	... or val in {'.', '..'} added in all 3 files
O1: length bound preserved (1–100 chars)	✅	1 leading char + {0,99} trailing = 1–100, matches old {1,100}
O1: all 3 octo-*.yml updated consistently	✅	Identical regex in all three call sites
O5: null guard on issue.user	✅	issue-welcome.yml:28 — if (issue.user && issue.user.type === 'Bot')
Path-traversal made impossible by construction	✅	. and .. can no longer pass validation; URL composition at octo-ci-status.yml:111,128,133 is now safe by construction

2. Findings

P2 — .github repo itself cannot use these reusable workflows

The new regex requires a leading alphanumeric, which rejects .github as a valid REPO_NAME. Since GitHub does allow the special .github repo name (it's literally the name of this repo), any future caller wanting to notify about events in the .github repo would be rejected.

In practice this is unlikely to matter — these workflows are typically invoked from feature repos (octo-server, octo-deployment, etc.), not the meta-repo. But it's worth being aware of: the trade-off here is "path-traversal-impossible by construction" vs. "supports the .github edge case." The PR explicitly chose the former, which is the right call for a defense-in-depth security control.

If supporting .github ever becomes needed, the cleaner fix would be an explicit allowlist of leading-dot exceptions rather than loosening the regex.

Nit — val in {'.', '..'} is redundant with the new regex

The leading-alphanumeric requirement already rejects both . (single char fails [A-Za-z0-9]) and .. (first . fails [A-Za-z0-9]). The explicit set check is defensive belt-and-suspenders — not harmful, and arguably good for making intent obvious to the next reader. Leave as-is.

O5 semantics — null user treated as non-bot ✅

if (issue.user && issue.user.type === 'Bot') causes ghost/deleted-account issues to fall through to the welcome path rather than being skipped as bots. That's the right default: a deleted human account opening an issue should still get the welcome message logic (which itself handles missing user gracefully downstream, since the body composition doesn't depend on issue.user).

3. Suggestions

• None blocking. Consider adding a short inline comment to the regex itself explaining the leading-alphanumeric requirement is intentional anti-traversal (the function docstring covers it, but the regex looks fiddly enough that a future reader may "simplify" it back).

4. Additional observations (out of scope)

• octo-pr-feed.yml:111,128,133 and equivalents: the hardcoded org Mininglamp-OSS/{repo} in the URL is the second layer of defense — even if repo validation were ever bypassed, the org prefix prevents arbitrary owner/repo injection. Good defense-in-depth.
• require_group_id (octo-pr-feed.yml:101 etc.) correctly notes "this is not an authorization check" — purely format validation. No action needed; just calling out that the comment is well-written and accurate.

[Jerry-Xin]

Project relevance gate: passed. This PR updates reusable GitHub workflows that are part of Mininglamp-OSS/.github.

No blocking issues found.

💬 Non-blocking:

• 🔵 Suggestion: In .github/workflows/octo-ci-status.yml:85, .github/workflows/octo-issue-feed.yml:93, and .github/workflows/octo-pr-feed.yml:110, or val in {'.', '..'} is now redundant because the new regex requires the first character to be alphanumeric. Keeping it is harmless and documents intent, so this is not a merge concern.
• 🔵 Suggestion: There is no test harness for these embedded workflow scripts. If this repository later adds workflow-script tests, require_repo_name() should get regression cases for ., .., slash-containing names, max length, and ordinary dotted repo names.

✅ Highlights:

• The stricter repo-name regex prevents . / .. and slash/path traversal inputs by construction while preserving normal repo names.
• .github/workflows/issue-welcome.yml:28 correctly guards issue.user before reading .type, avoiding a nullable webhook payload edge case.
• The change is consistent across all three copied require_repo_name() implementations.

[lml2468]

Summary

Tightens require_repo_name regex to require leading alphanumeric (preventing ./.. path traversal by construction) and adds null guard on issue.user in the welcome workflow.

Findings

No new blocking issues. Verified at head 8cdb9b4e:

• Regex correctness: [A-Za-z0-9][A-Za-z0-9._-]{0,99} preserves 1-100 char length bound, rejects ./.. by construction. Applied consistently across all 3 octo-*.yml files.
• Null guard: issue.user && issue.user.type === 'Bot' correctly handles ghost/deleted accounts — they fall through to the welcome path, which is the right default.
• Redundant belt-and-suspenders: val in {'.', '..'} is made unreachable by the leading-alphanumeric requirement, but documents intent. Harmless, keep as-is.
• CI: add-to-project failure is pre-existing (missing PROJECT_TOKEN secret on this repo) — not introduced by this PR. The two new sanity checks pass.

Prior reviewers (yujiawei, Jerry-Xin) already noted the .github repo name edge case (P2, non-blocking). I agree with their assessment — the security trade-off is correct.

Verdict

APPROVED — small, focused, correct. No duplicated findings. (Submitted as COMMENTED due to GitHub self-approval limitation.)