fullsend issues post-comment: findMarkedTrackerComment should filter by comment author to prevent marker spoofing

[fullsend-ai-retro]

What happened

PR #6144 added fullsend issues get and fullsend issues post-comment commands with a new findMarkedTrackerComment function (issues.go L427-437) that searches all comments for a marker substring without filtering by author. The existing GitHub-specific sticky.FindMarkedComment (sticky.go L90-104) takes a botUser parameter and skips comments from other authors, with an explicit test (TestFindMarkedComment_IgnoresSpoofedMarker) verifying that spoofed markers from non-bot users are ignored.

During review, fullsend-ai-review[bot] flagged this as a [low] finding noting the trust-model tradeoff. The human reviewer (ralphbean) acknowledged the gap and explicitly requested a follow-up issue: "a note to retro agent - I want an issue filed to followup on this after this is merged." The code includes an inline comment documenting the gap: tracker.Client has no GetAuthenticatedUser method.

What could go better

Without author filtering, any user with comment-write access on a GitHub/GitLab/Jira issue could post a comment containing the marker string before the bot runs. On the next invocation, findMarkedTrackerComment would match the attacker's comment, and the bot would update it instead of creating or managing its own. This could be used to inject content into what appears to be a bot comment, or to redirect bot updates to a comment controlled by the attacker.

The current mitigation is a documented trust assumption (CI environments with controlled access). This is reasonable for the initial release but creates a gap compared to the GitHub-specific sticky path, which has defense-in-depth via botUser filtering. As fullsend issues post-comment is adopted more broadly (including potentially by third-party callers), the trust assumption weakens.

Confidence: High — the code gap is clear and the human reviewer explicitly confirmed it warrants a follow-up.

Proposed change

Close the gap by adding authenticated-user resolution to the tracker comment path:

1. Add a GetAuthenticatedUser(ctx) (string, error) method to tracker.Client (or a narrower interface). The GitHub and GitLab implementations can delegate to their respective forge clients. The Jira implementation can derive the user from the email used for authentication.

2. Update findMarkedTrackerComment to accept a botUser string parameter and skip comments whose Author field doesn't match, mirroring sticky.FindMarkedComment.

3. Update postTrackerStickyComment to call GetAuthenticatedUser and pass the result to findMarkedTrackerComment. If the call fails, degrade gracefully (log a warning and fall back to unfiltered search) rather than blocking the command.

4. Add a test analogous to TestFindMarkedComment_IgnoresSpoofedMarker — verify that when a non-bot comment contains the marker, the function returns only the bot's comment.

Validation criteria

1. A new test TestFindMarkedTrackerComment_IgnoresSpoofedMarker passes, demonstrating that comments from non-bot users containing the marker string are skipped.
2. The existing TestRunIssuesPostComment_* tests continue to pass with the updated signature.
3. Manual verification: running fullsend issues post-comment --tracker github on a repo where a non-bot comment contains the marker string results in the bot creating its own comment rather than updating the spoofed one.

---

Generated by retro agent from #6144

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 5:12 PM UTC · Completed 5:16 PM UTC

Commit: 290c351 · View workflow run →

[fullsend-ai-triage]

Triage Summary

Severity: Medium — defense-in-depth gap; current trusted-CI deployment model mitigates the risk, but the gap widens as adoption broadens.

Related work: Issue #2090 covers authenticated-user resolution failures in the review post-script (different component). While thematically related, it is not a blocker — tracker.Client.GetAuthenticatedUser can be added independently since forge.Client already exposes the method and the ForgeClient adapter can delegate directly.

Proposed test

func TestFindMarkedTrackerComment_IgnoresSpoofedMarker(t *testing.T) {
    marker := "<!-- fullsend:test-agent -->"
    botUser := "fullsend-ai-triage[bot]"
    comments := []tracker.Comment{
        {ID: "1", Author: "malicious-user", Body: tracker.Body(marker + "\nspoofed")},
        {ID: "2", Author: botUser, Body: tracker.Body(marker + "\nlegit")},
    }
    result := findMarkedTrackerComment(comments, marker, botUser)
    assert.Equal(t, "2", result.ID)
}

func TestFindMarkedTrackerComment_FallsBackWhenBotUserEmpty(t *testing.T) {
    marker := "<!-- fullsend:test-agent -->"
    comments := []tracker.Comment{
        {ID: "1", Author: "any-user", Body: tracker.Body(marker + "\ncontent")},
    }
    result := findMarkedTrackerComment(comments, marker, "")
    assert.Equal(t, "1", result.ID)
}

---

Labels: This is a security hardening issue (marker spoofing prevention) at medium priority — the gap is documented and mitigated in current deployments but should be closed for defense-in-depth. The component is the harness/CLI tracker integration.

---

Next steps:

• /fs-code — agent creates a PR to implement this issue
• /fs-code <your instruction> — agent implements with your specific guidance