Skip to content

fix: escape untrusted attachment names in prompt text - #106

Closed
ladydd wants to merge 1 commit into
LodyAI:mainfrom
ladydd:fix/escape-attachment-file-names
Closed

fix: escape untrusted attachment names in prompt text#106
ladydd wants to merge 1 commit into
LodyAI:mainfrom
ladydd:fix/escape-attachment-file-names

Conversation

@ladydd

@ladydd ladydd commented Aug 28, 2026

Copy link
Copy Markdown

Author type

  • I am an Agent (check this if an LLM agent authored this PR)
  • I am a human

Problem / pressure

Attachment display names are stored as unconstrained strings and interpolated into the text given to the agent ([User sent a file: …]). A name with ] or a line break changes the shape of that sentence. Names are untrusted caller input (upload File.name, path basename). On-disk sanitizeAttachmentFileName already applies to the saved path only.

Summary

formatUntrustedFileNameForPrompt escapes ] and backslashes and collapses line breaks. Used by buildAttachmentPromptText and buildUnavailableAttachmentPromptText. Saved attachment paths are unchanged.

Companion (required, other repo): ladydd/acp-extension-codex fix/escape-resource-link-filenames (32037d7, against LodyAI/acp-extension-codex). That PR is the markdown-label half (escapeMarkdownLinkLabel in both formatUriAsLink implementations). ACP resource_link.name stays the raw display name in this PR so that adapter can escape it at the markdown boundary. Open that PR together with this one.

Before / after

Before After
Display name copied into the prompt verbatim ] escaped; newlines flattened to spaces
build.log Unchanged

Test plan

On this branch (fix/escape-attachment-file-names @ fd0f1dc), after pnpm install at the repo root:

corepack pnpm --filter lody exec vitest run src/lib/session-file-attachments.test.ts

Result: 22 passed.

Skipped: full apps/cli suite; e2e.

Agent handoff

Instructions for reviewing agents

  • Review focus: formatUntrustedFileNameForPrompt and the two prompt builders in session-file-attachments.ts.
  • Decisions to challenge: Escaping prompt text only; ACP resource_link.name is left as the raw display name for the adapter PR (ladydd/acp-extension-codex fix/escape-resource-link-filenames @ 32037d7) to escape.
  • Plausible failures / evidence gaps: Only the attachments unit file was run, not the full CLI suite.

Authoring context

  • User goal / directives: Keep attachment prompt lines stable when the display name has brackets or line breaks.
  • Constraints / non-goals: Do not change on-disk sanitizeAttachmentFileName. Do not put file bytes in the prompt.
  • Risk-bearing decisions: Prompt-visible name spelling for unusual characters.
  • Destructive or irreversible behavior: None.
  • Deliberately not done or tested: Full CLI suite.
  • Unknowns / confidence: High for the unit cases; other prompt construction paths still pass the raw fileName into ACP resource_link.name for the adapter to escape.

Sharing consent (author side)

Declining context sharing is respected, but it does not guarantee review. If withheld context prevents maintainers from assessing provenance, scope, or risk, they may decline the contribution or close the pull request.

  • Author-side user explicitly allowed publishing the Authoring context above
  • Author-side user explicitly declined publishing Authoring context and understands that maintainers may decline or close the contribution; keep every field as N/A / redacted

Display names are caller-supplied and currently interpolated into the
agent prompt as-is. Escape brackets and collapse line breaks so the
prompt stays a single description. On-disk sanitization is unchanged.
@github-actions

Copy link
Copy Markdown

@ladydd, this pull request body does not match Lody's PR template.

This PR is marked status:needs-pr-body. Update the description and satisfy every required section by 2026-09-04 03:59:16 UTC. The label and this comment are removed automatically after the body passes validation.

If the body remains invalid for 7 days, this PR will be closed and marked status:pr-body-expired. To contribute after that, open a new pull request using the current template.

Every external PR must link a Lody issue and provide a complete public Context handoff with concise, PR-specific review instructions. N/A and redacted context are not accepted because maintainers need enough provenance, scope, and risk information to assess the contribution.

Checker findings
PR body does not match the Lody pull request template:

- Missing required heading: ## Related issue
- Missing required heading: ## Context handoff
- Context handoff must keep <!-- context-handoff:begin/end --> markers.

See `.github/PULL_REQUEST_TEMPLATE.md`.

@github-actions github-actions Bot added the status:needs-pr-body PR body does not meet the contribution template label Aug 28, 2026
@ladydd

ladydd commented Aug 28, 2026

Copy link
Copy Markdown
Author

Companion PR is now open: LodyAI/acp-extension-codex#27

@wibus-wee

Copy link
Copy Markdown
Member

Closing this PR. This PR is being closed for scope, product fit, and lack of demonstrated value.

The proposed change is not accepted as a security fix and does not address a sufficiently demonstrated user-facing problem. The submission appears to be AI-generated and insufficiently reviewed, escalating speculative edge cases into security claims and introducing user-facing behavior without prior product approval.

Any further automated or unsolicited submissions on this topic, or similar submissions without prior maintainer approval, will result in this account being blocked from contributing to the project.

If I have misunderstood the intent, impact, or context of this change, please explain it here before opening any follow-up. I am willing to reconsider based on concrete reproduction steps, demonstrated user impact, and an approved product direction. Do not open companion or follow-up PRs before that discussion.

@wibus-wee wibus-wee closed this Aug 28, 2026
@wibus-wee wibus-wee added the invalid This doesn't seem right label Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

invalid This doesn't seem right scope: cli status:needs-pr-body PR body does not meet the contribution template

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants