Skip to content

Commit 2ceaf7e

Browse files
committed
docs(skill): stop routing template bodies through command substitution
Review of the previous commit found the safe-write flow it introduced carried the same class of defect it exists to prevent. Three corrections: Silent truncation. The flow moved channel bodies with `"$(cat …)"` / `"$(jq -r …)"`. Bash command substitution strips every trailing newline, so a body that legitimately ends in a blank line was written back shortened — and the verification step compared only which fields were non-empty, so a truncated-but-still-non-empty channel reported clean. Measured: a 15-byte body round-trips as 11. The write step now builds the whole request with `jq --rawfile` (byte-exact) and posts it via `--data -`, and step 5 diffs per-channel byte lengths instead of the non-empty field set, which catches truncation and wipes alike. Rebuilding the body from the snapshot also removes the previous "one flag per non-empty key" instruction, whose jq filter was unscoped and surfaced template_id/status/created_at/updated_at — keys `update` has no flags for. Miscount. `buildTemplateUpdates` writes 14 channel-content fields, not 16. Category error. `status` was listed among the inputs that survive omission. It is not a field of `update`'s request at all — it moves only through the separate enable/disable endpoints, which the CLI does not expose. The survivors are exactly the pointer-typed inputs: team_id, feishu_app_card_v2_table_enabled, incident_card_hidden_fields. Guard extended and mutation-verified.
1 parent 45c43bc commit 2ceaf7e

2 files changed

Lines changed: 54 additions & 27 deletions

File tree

internal/skilldoc/source_cards_test.go

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,11 @@ func TestTemplateCardUpdateSemanticsAreDestructive(t *testing.T) {
4141
for _, banned := range []string{
4242
"omitted channel flags are left unchanged",
4343
"only supplied fields overwrite",
44+
// buildTemplateUpdates writes 14 channel-content fields, not 16.
45+
"16 channel fields",
46+
// status is not a field of update's request at all, so it is not a
47+
// pointer-typed input that "survives" omission.
48+
"and `status` survive omission",
4449
} {
4550
if strings.Contains(text, banned) {
4651
t.Errorf("template card reasserts the retracted patch-semantics claim %q; update writes every channel field unconditionally", banned)
@@ -51,8 +56,17 @@ func TestTemplateCardUpdateSemanticsAreDestructive(t *testing.T) {
5156
"full-object replace",
5257
"is CLEARED",
5358
"survive omission",
59+
"14 channel-content fields",
5460
"info --json",
55-
"Verify the FIELD SET",
61+
// Length comparison, not a non-empty field-set check: only the former catches a
62+
// body that was truncated rather than cleared.
63+
"Verify LENGTHS",
64+
// The write path must move bodies with jq, never through command substitution,
65+
// which strips every trailing newline off a template body.
66+
"--rawfile",
67+
"--data -",
68+
`"$(cat`,
69+
"strips *all* trailing newlines",
5670
"--feishu-app-card-v2-table-enabled",
5771
} {
5872
if !strings.Contains(text, want) {

skills/flashduty/reference/template.md

Lines changed: 39 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,38 +57,43 @@ verify. Skipping step 1 or step 5 is how a live channel gets silently blanked.
5757

5858
```bash
5959
T=<template-id> # POSITIONAL on update/info/delete; --template-name always required
60+
CHLEN='["dingtalk","dingtalk_app","email","feishu","feishu_app","slack","slack_app","sms","teams_app","telegram","voice","wecom","wecom_app","zoom"] as $ch | . as $t | $ch[] | "\(.)\t\($t[.] // "" | length)"'
6061

6162
# 1. Snapshot the whole template — this is both your backup and your write payload
6263
fduty template info "$T" --json > /tmp/tpl.json
63-
NONEMPTY='to_entries[]|select(.value|type=="string")|select(.value!="")|.key'
64-
jq -r "$NONEMPTY" /tmp/tpl.json # the channels that must survive this edit
64+
jq -r "$CHLEN" /tmp/tpl.json # every channel and its current byte length
6565

6666
# 2. Edit only the channel you care about, on disk
6767
jq -r '.feishu_app' /tmp/tpl.json > /tmp/feishu_app.tpl
6868
# …edit /tmp/feishu_app.tpl…
6969

7070
# 3. Preview the edited source against a REAL incident before writing
71-
fduty template preview --type feishu_app --content "$(cat /tmp/feishu_app.tpl)" \
72-
--incident-id <incident-id>
73-
74-
# 4. Write — your edit PLUS every other channel that was non-empty in the snapshot
75-
fduty template update "$T" \
76-
--template-name "$(jq -r '.template_name // ""' /tmp/tpl.json)" \
77-
--description "$(jq -r '.description // ""' /tmp/tpl.json)" \
78-
--feishu-app "$(cat /tmp/feishu_app.tpl)" \
79-
--dingtalk-app "$(jq -r '.dingtalk_app // ""' /tmp/tpl.json)" \
80-
--dingtalk "$(jq -r '.dingtalk // ""' /tmp/tpl.json)"
81-
# …one flag per non-empty key from step 1; omitting any of them clears it
82-
83-
# 5. Verify the FIELD SET, not just your edit
71+
jq -n --rawfile c /tmp/feishu_app.tpl \
72+
'{type:"feishu_app", content:$c, incident_id:"<incident-id>"}' \
73+
| fduty template preview --data -
74+
75+
# 4. Write — rebuild the body from the snapshot, moving template bodies with jq and
76+
# NEVER through "$(...)": command substitution strips every trailing newline, so a
77+
# body ending in a blank line would come back silently shortened.
78+
jq -c --rawfile feishu_app /tmp/feishu_app.tpl \
79+
'{template_id, template_name, description,
80+
dingtalk, dingtalk_app, email, feishu, feishu_app, slack, slack_app, sms,
81+
teams_app, telegram, voice, wecom, wecom_app, zoom}
82+
| .feishu_app = $feishu_app' /tmp/tpl.json \
83+
| fduty template update --data -
84+
# Everything left out of that object is patch-semantics and survives untouched:
85+
# team_id, feishu_app_card_v2_table_enabled, incident_card_hidden_fields.
86+
87+
# 5. Verify LENGTHS, not just the field you edited
8488
fduty template info "$T" --json > /tmp/tpl_after.json
85-
diff <(jq -r "$NONEMPTY" /tmp/tpl.json | sort) <(jq -r "$NONEMPTY" /tmp/tpl_after.json | sort)
86-
# empty diff = nothing was wiped. A key only on the left = you just deleted a live
87-
# channel; restore it immediately from /tmp/tpl.json.
89+
diff <(jq -r "$CHLEN" /tmp/tpl.json) <(jq -r "$CHLEN" /tmp/tpl_after.json)
90+
# Only the channel you edited may differ. A channel that dropped to 0 was wiped; a
91+
# channel a few bytes shorter was truncated — restore it from /tmp/tpl.json.
8892
```
8993

9094
Checking only the field you edited is **not** verification — the damage from a full-object
91-
replace always lands on the fields you did not touch.
95+
replace always lands on the fields you did not touch, and comparing only *which* fields are
96+
non-empty misses a body that was shortened rather than cleared.
9297

9398
<!-- GENERATED:template START · 由 fduty __dump-commands 同步 · 勿手改 fence 内 -->
9499

@@ -202,13 +207,21 @@ Note: `create` / `update` flags use **hyphenated** names (`--dingtalk-app`, `--f
202207

203208
- **`info`, `update`, `delete` take `<template-id>` as a positional first argument** — pass it bare, not as `--template-id`. `create`, `list`, `preview`, `validate`, `get-preset`, `functions`, `variables` take all inputs as flags.
204209
- **`update` is a full-object replace — every channel field you omit is CLEARED.** The
205-
server writes all 16 channel fields plus `description` on every call, and a field absent
206-
from the request arrives as the empty string: omitting `--dingtalk-app` sets
207-
`dingtalk_app` to `""`, and that channel silently stops rendering for every escalation
208-
rule bound to the template. Only `team_id`, `feishu_app_card_v2_table_enabled`,
209-
`incident_card_hidden_fields` and `status` survive omission (they are patch-semantics).
210-
Always snapshot with `info --json` first and pass every non-empty channel back — see the
211-
hot flow above. `--template-name` is required on every update even when unchanged.
210+
server writes all 14 channel-content fields plus `description` on every call, and a
211+
field absent from the request arrives as the empty string: omitting `--dingtalk-app`
212+
sets `dingtalk_app` to `""`, and that channel silently stops rendering for every
213+
escalation rule bound to the template. Only the pointer-typed inputs survive omission:
214+
`team_id`, `feishu_app_card_v2_table_enabled`, `incident_card_hidden_fields`. (`status`
215+
is not part of `update`'s request at all — it moves only through the separate
216+
enable/disable endpoints, which the CLI does not expose — so `update` can never change
217+
it.) Always snapshot with `info --json` first and rebuild the body from that snapshot —
218+
see the hot flow above. `--template-name` is required on every update even when
219+
unchanged.
220+
- **Never move a template body through `"$(cat …)"` or `"$(jq -r …)"`.** Bash command
221+
substitution strips *all* trailing newlines, so a body that legitimately ends in a blank
222+
line is written back shortened — and a check that only asks which fields are non-empty
223+
cannot see it, because the field is still non-empty. Carry bodies with `jq --rawfile`
224+
and write with `--data -`, as the hot flow does.
212225
- **`--feishu-app-card-v2-table-enabled` uses pointer semantics on `update`** — unlike the plain string channel-content flags, it patches the table-rendering setting only when the flag is explicitly passed; omit it to leave the existing setting untouched. It is a plain bool on `create` (no prior setting to preserve).
213226
- **`list` returns every channel's full template source for every row** — a few dozen
214227
templates blow past a tool-output cap in one call. Never render it directly: go to a

0 commit comments

Comments
 (0)