chore: repository foundations, Marketplace branding, and security threat model - #61
chore: repository foundations, Marketplace branding, and security threat model#61Benkapner wants to merge 4 commits into
Conversation
|
🤖 Finished Review · ✅ Success · Started 11:36 AM UTC · Completed 11:53 AM UTC Commit: |
Phase 0: Repository foundations. The repo has SECURITY.md and RELEASING.md but no contributor onboarding guide despite seven forks. Add CONTRIBUTING.md with dev setup, testing, lint, commit conventions, and PR expectations. Add structured issue templates that capture model backend, model name, docs format, repo setup, and trigger command, which together determine almost every bug's reproduction path. Add a PR template with a testing checklist.
Phase 0: Repository foundations. A project that asks users to grant contents:write should model good supply-chain hygiene. The CI workflow (actions/checkout, astral-sh/setup-uv) was already pinned to commit SHAs. The fullsend reusable workflow uses a moving v0 tag coordinated with fullsend_ai_ref; added a comment documenting this constraint. Dependabot was already configured for both github-actions and pip ecosystems with weekly updates.
Phase 1: Releases and versioning. Fill out action.yml branding and metadata for Marketplace listing. Update name from placeholder to match the repo name, set author to the org, use book-open icon and red color to match Red Hat branding. The actual Marketplace publish is a manual UI action on the Release page, documented in RELEASING.md.
The README warns about Jira content leaking out but says nothing about untrusted content coming in. Documentation content, code diffs, and external system content all flow into prompts, and on fork PRs much of that input is contributor-controlled. Extend SECURITY.md with a threat model enumerating every untrusted input, what it can influence, and the current mitigations. Add explicit guidance on fork PRs, including what the author_association gate does and does not protect against. Note the contents:write implication. Cross-link from the README.
f8c76db to
d2d9991
Compare
ReviewFindingsHigh
Medium
Low
Labels: PR adds documentation files (CONTRIBUTING.md, SECURITY.md threat model) and modifies GitHub Actions templates/workflow Next steps:
Previous runReviewReason: stale-head The review agent reviewed commit |
|
/fs-review |
|
🤖 Finished Review · ✅ Success · Started 11:54 AM UTC · Completed 12:12 PM UTC Commit: |
![fullsend-ai-review[bot]](https://avatars.githubusercontent.com/in/3478501?s=60&v=4){kind=small}
| contains(fromJSON('["OWNER","MEMBER","COLLABORATOR"]'), github.event.comment.author_association) | ||
| ``` | ||
|
|
||
| This prevents external contributors from triggering the action on their own |
There was a problem hiding this comment.
[medium] Technical documentation accuracy
The author_association gate in the threat model states only OWNER, MEMBER, and COLLABORATOR can trigger commands. The stop-fix job in fullsend.yaml uses a broader gate that also permits CONTRIBUTOR and the PR author. While stop-fix gates a different command (/fs-fix-stop) than the code-to-docs commands, the discrepancy between the documented gate and the repository's own workflow should be clarified.
Suggested fix: Add a note clarifying that the stop-fix job uses a broader gate, or document that the three-role gate applies specifically to code-to-docs command triggers.
| Documentation content and code diffs are included in LLM prompts. A | ||
| malicious document containing text like "ignore previous instructions" could | ||
| theoretically influence the LLM's output. Mitigations: | ||
|
|
There was a problem hiding this comment.
[medium] Technical documentation accuracy
LLM Prompt Injection mitigation #1 claims document content is wrapped in explicit BEGIN/END delimiters with untrusted-data labeling. No such delimiters exist in the codebase. PR #53 (referenced as adding this feature) is still open.
Suggested fix: Either wait for PR #53 to merge before claiming these mitigations exist, or rewrite mitigations 1-2 as planned/forthcoming rather than current.
| theoretically influence the LLM's output. Mitigations: | ||
|
|
||
| 1. Document content in verification prompts is wrapped in explicit | ||
| `--- BEGIN/END ... (untrusted content, data only) ---` delimiters with |
There was a problem hiding this comment.
[medium] Technical documentation accuracy
Mitigation #2 claims post-generation validation checks for large content removals and verifies updates with an independent LLM call. No such code exists. The only post-generation validation is format parsing.
Suggested fix: Remove or correct this claim to reflect actual mitigations, or label as planned functionality.
| | `.code-to-docs/style.md` | Base branch (maintainer-controlled) | Injected into generation prompts as style guidelines | Loaded from the base branch, not the PR branch. | | ||
| | `.code-to-docs/config.json` | Base branch (maintainer-controlled) | Controls validation thresholds and behavior | Loaded from the base branch, not the PR branch. Values are validated on load. | | ||
|
|
||
| ### Fork PRs |
There was a problem hiding this comment.
[low] Technical documentation accuracy
The untrusted inputs table states .code-to-docs/config.json 'Values are validated on load.' In reality, load_repo_config() performs JSON parsing but no schema, type, or range validation.
Suggested fix: Change to a more accurate description such as 'Parsed as JSON; individual consumers validate their own fields.'
fullsend-ai-review
Bot
added
documentation
1 participant
Summary
Zero-risk repository housekeeping. No tool behavior changes, no code paths modified.
Test plan
uv run pytest -vpasses (429 tests, no changes to test or source code)uv run ruff check src/ tests/anduv run ruff format --check src/ tests/cleanThis is the first of several PRs splitting the improvement roadmap into reviewable pieces. It covers Tier 1 (zero-risk, no behavior changes). Depends on PR #53 being merged first.