Skip to content

docs: add mysql credential hygiene (passwords not in argv) guide#121

Open
weicao wants to merge 1 commit into
mainfrom
feature/addon-mysql-credential-hygiene
Open

docs: add mysql credential hygiene (passwords not in argv) guide#121
weicao wants to merge 1 commit into
mainfrom
feature/addon-mysql-credential-hygiene

Conversation

@weicao
Copy link
Copy Markdown
Contributor

@weicao weicao commented May 13, 2026

Summary

New methodology doc addon-mysql-credential-hygiene-no-argv-guide.md covering when addon lifecycle action / ActionSet hook / test script calls mysql obclient and other MySQL-protocol clients, the password must not appear in argv.

Body (generic methodology, version-agnostic, no engine binding):

  • Why passwords in argv leak (ps / /proc/cmdline / pod describe / log aggregation / E2E archives)
  • Forbidden argv forms (-p$VAR, --password=$VAR, MYSQL_PWD env fallback)
  • Allowed forms: stdin or temp client config (mktemp -d + chmod 700 + umask 077 + trap 'rm -rf \"$tmp_dir\"' EXIT INT TERM + --defaults-file + outer timeout -k 1 N)
  • patch-gate 3 indicators (unsafe_mysql_argv_matches=0 / has_timeout=1 / has_trap_cleanup=1) only prove fix is in live ActionSet, NOT product / functional PASS / acceptance / release-ready
  • 5-point PR review checklist
  • 3 反例 vs 正例 对照
  • Relation to addon-evidence-discipline-guide.md / addon-bounded-eventual-convergence-guide.md / addon-test-acceptance-and-first-blocker-guide.md

Appendix A is OceanBase enterprise addon oceanbase-physical-backup postReady case (live sha b74912857084451adbf707166a82bd8f55efce5ca74f3b2ce964c65451fc9925) with explicit boundary statement: 6-sample observation across two runtime paths supports only that the three indicators were 0 / 1 / 1 in those samples; does NOT extrapolate to PITR full coverage / addon acceptance / release-ready.

SKILL-INDEX.md updated: added entry under section 1 (设计 / 开发新 addon) with single-line concise description, same wording-discipline as existing entries.

Test plan

  • Manual: read full body, confirm methodology generic (no OB-specific commands in body)
  • Manual: confirm Appendix A is the only OceanBase-specific section and contains explicit boundary statement
  • Manual: SKILL-INDEX entry text matches doc body summary
  • Manual: cross-doc references resolve

Methodology body covers:
- Why passwords in argv leak via ps / /proc/cmdline / events / log aggregation
- Forbidden argv forms (-p$VAR, --password=$VAR, MYSQL_PWD environment)
- Allowed forms: stdin or temp client config + chmod 700 + umask 077 + trap
  rm -rf "$tmp_dir" EXIT INT TERM + --defaults-file + bounded timeout
- patch-gate 3 indicators (unsafe_mysql_argv_matches=0 / has_timeout=1 /
  has_trap_cleanup=1) only prove fix in place, NOT product / acceptance /
  release-ready
- 5-point PR review checklist

Appendix A is OceanBase enterprise addon oceanbase-physical-backup postReady
case (live sha b74912857084451adbf707166a82bd8f55efce5ca74f3b2ce964c65451fc9925)
with explicit "do not extrapolate" boundary; 6-sample observation across two
runtime paths supports only that the three indicators were observed live and
counted as 0 / 1 / 1.
@weicao
Copy link
Copy Markdown
Contributor Author

weicao commented May 13, 2026

Blocking review before merge.

The topic is useful and should land after cleanup, but this PR is not public-ready yet.

  1. PR body has forbidden public attribution/provenance wording.

    • Remove the Generated with Claude Code footer.
    • Remove AI / agent wording from the test plan.
    • Remove person-specific review wording such as per Allen review guidance; use role-neutral wording like per docs review guidance.
  2. The guide intro is missing the standard fifth field.
    Add > **Affected by version skew**: ... after Applies to KB version.
    Suggested wording:
    yes — MySQL-protocol client implementations differ in how they handle password prompts, option files, and environment variables; the argv hygiene rule is stable, but client-specific alternatives must be verified per binary/version.

  3. The generic “stdin” password workaround is unsafe as written.
    The example printf '%s\n' "$OB_ROOT_PASSWD" | mysql ... --skip-password does not prove a portable MySQL password-from-stdin path; --skip-password means the client will not use a password. Please remove stdin as a generic allowed password-passing form from the main methodology, or move it to an appendix only when a specific client has verified --password-stdin-style support. For the main body, make temporary client config with --defaults-file the generic recommended path.

  4. After changing [style-harmonize] cross-ref hygiene — broken link + stale annotation #3, update PR body and SKILL-INDEX wording so they no longer say “stdin or temp client config” as two equal allowed forms. The index should say the generic safe path is temp client config plus permissions, cleanup, and timeout.

  5. Re-run the hygiene checks after the rewrite:

    • git diff --check
    • new guide 5-field intro
    • SKILL-INDEX entry still matches the guide
    • PR body + commit body attribution grep clean
    • changed-file local links clean

Evidence from my check: git diff --check is clean and the branch is mergeable, but the public attribution footer and missing intro field are blockers, and the stdin password pattern needs correction before this becomes reusable guidance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant