Skip to content

fix(ui): quote YAML scalars containing embedded newlines - #7820

Merged
JSONbored merged 1 commit into
JSONbored:mainfrom
jaytbarimbao-collab:fix-yaml-newline-7786
Jul 21, 2026
Merged

fix(ui): quote YAML scalars containing embedded newlines#7820
JSONbored merged 1 commit into
JSONbored:mainfrom
jaytbarimbao-collab:fix-yaml-newline-7786

Conversation

@jaytbarimbao-collab

Copy link
Copy Markdown
Contributor

Closes #7786

yamlScalar (apps/loopover-ui/src/lib/config-generator-yaml.ts) decides a value needs quoting from value.trim() !== value plus a special-character regex:

const needsQuote = value === "" || value.trim() !== value || /[:#[\]{}&*!|>'\"%@`,]/.test(value);

trim() only strips edge whitespace, and the regex omits line breaks — so a value with an embedded newline and none of the flagged characters (e.g. "claude 3.7\nignore_findings true") is returned unquoted. formStateToYaml then emits it as a raw multi-line scalar: a stray top-level line that breaks the generated .loopover.yml, violating the file's documented invariant that a fresh/partial form never produces invalid output. This is the same incomplete-guard bug class as the already-fixed CSV formula-injection gap in csv-export.ts.

Fix: add \n and \r to the needsQuote regex so any line-break-containing value goes through the existing JSON.stringify quoting path. No call-site or signature change.

Tests: adds embedded-newline and carriage-return cases to config-generator-yaml.test.ts (which previously covered colon and edge-whitespace but not line breaks), including an assertion that the serialized output stays a single line. Verified locally: eslint + prettier clean, tsc --noEmit clean, 11/11 tests pass.

Note: currently latent/defense-in-depth — the sole live call site binds model to a plain <input type=text>, which strips line breaks before onChange. It becomes reachable the moment any field reusing yamlScalar is widened to a <textarea> or populated programmatically. (apps/loopover-ui is outside the src/** 99% patch gate; the test case is this issue's own deliverable.)

yamlScalar decided a value needs quoting from value.trim() !== value plus
a special-character regex. trim() only strips edge whitespace, and the
regex omitted line breaks, so a value with an embedded newline and no other
flagged character (e.g. "claude 3.7\nignore_findings true") was emitted as
a raw multi-line scalar -- a stray top-level line that breaks the generated
.loopover.yml, violating the file's "a partial form never produces invalid
output" invariant. Same incomplete-guard bug class as the csv-export fix.

Add \n and \r to the needsQuote regex so any line-break-containing value
goes through the existing JSON.stringify quoting path. Adds embedded-newline
and carriage-return regression cases to config-generator-yaml.test.ts.

Closes JSONbored#7786
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 21, 2026
@loopover-orb

loopover-orb Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Tip

✅ LoopOver review result - approve/merge recommended

Review updated: 2026-07-21 13:59:18 UTC

2 files · 1 AI reviewer · no blockers · readiness 98/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
Fixes a real gap: `yamlScalar`'s `needsQuote` check missed embedded `\n`/`\r`, so a value like a model name with a stray newline would be emitted as an unquoted, file-breaking raw multi-line YAML scalar. The fix (adding `\n\r` to the regex) is correct, minimal, and routes through the existing `JSON.stringify` quoting path with no signature/call-site change. Tests cover both the newline and carriage-return cases and assert the output stays single-line, closing the gap described in #7786. The description itself acknowledges this is currently latent (the sole call site is a plain `<input>` that already strips newlines) but correctly treats it as defense-in-depth against future reuse (e.g. a `<textarea>`).

Nits — 3 non-blocking
  • The external brief's claim that `Config-generator YAML scalar escaping doesn't guard embedded newlines #7786` in a comment is a 'magic number' is a false positive — it's an issue reference in a comment, not a numeric literal used in logic.
  • config-generator-yaml.ts:30 comment is fairly long for a one-line regex change; could be trimmed, though the historical context is genuinely useful here.
  • Consider also testing a value with only a leading/trailing newline (e.g. `"\nclaude"`) to confirm it's still caught by the existing `value.trim() !== value` branch rather than relying on the new regex arm.

Decision drivers

  • ✅ Code review — No blockers (1 reviewer)
  • ✅ Gate result — Passing (No configured blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #7786
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 151 registered-repo PR(s), 73 merged, 16 issue(s).
Contributor context ✅ Confirmed Gittensor contributor jaytbarimbao-collab; Gittensor profile; 151 PR(s), 16 issue(s).
Improvement ✅ Minor risk: clean · value: minor
Linked issue satisfaction

Addressed
The regex now includes \n and \r so any value with an embedded line break is quoted via JSON.stringify, matching the requested fix, and new tests cover embedded newline and carriage return cases as required.

Review context
  • Author: jaytbarimbao-collab
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: not available
  • Official Gittensor activity: 151 PR(s), 16 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Keep the PR focused and include validation evidence before maintainer review.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.

  • @loopover ask &lt;question&gt; answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat &lt;question&gt; answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

Full command reference: https://loopover.ai/docs/loopover-commands

🧪 Experimental — new and may change.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.

  • Re-run LoopOver review

@loopover-orb loopover-orb Bot added the manual-review Gittensor contributor context label Jul 21, 2026
@JSONbored
JSONbored merged commit a811c12 into JSONbored:main Jul 21, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Config-generator YAML scalar escaping doesn't guard embedded newlines

2 participants