test(doc): harden markdown_fix pipeline with invariant tests#1
Closed
fangshuyu-768 wants to merge 7 commits intoGaoSSR:fix/docs-fetch-roundtrip-formattingfrom
Closed
Conversation
* fix: preserve attachment metadata on base uploads * test: cover attachment mime detection * fix: address attachment upload review feedback * fix: preserve source extension for attachment mime detection * fix: avoid registry test refresh data race * Revert "fix: avoid registry test refresh data race" This reverts commit c1d12d0. --------- Co-authored-by: kongenpei <kongenpei@users.noreply.github.com>
* feat(sidecar): add sidecar proxy for sandbox credential isolation
Keep real secrets (app_secret, access_token) out of sandbox environments.
CLI instances inside sandboxes connect to a trusted sidecar process via
HTTP; the sidecar verifies HMAC-signed requests and injects real tokens
before forwarding to the Lark API.
Key components:
- `auth proxy` subcommand to start the sidecar server (build tag: authsidecar)
- Noop credential provider returns sentinel tokens in sidecar mode
- Transport interceptor rewrites requests to sidecar with HMAC signature
- Env provider yields to sidecar provider when AUTH_PROXY is set
- Supports both feishu and lark brand endpoints
* feat(sidecar): implement priority ordering for credential providers
* feat(sidecar): strip client-supplied auth headers and improve shutdown logging
* feat(sidecar): buffer request body to prevent HMAC mismatches on read errors
* feat(sidecar): fix CI
* refactor(sidecar): publish protocol package and move server to reference demo
The sidecar server is no longer shipped as a `lark-cli auth proxy`
subcommand. Instead, the CLI provides only the standard sidecar *client*
(via `-tags authsidecar`), while the wire-protocol utilities are exposed
as a public package for integrators to implement their own server.
Changes:
- Move `internal/sidecar/` → `sidecar/` so external integrators can
import HMAC signing, headers, sentinels and address validators.
- Remove `cmd/auth/proxy.go`, `proxy_stub.go`, `proxy_test.go` and the
conditional registration in `cmd/auth/auth.go`.
- Add `sidecar/server-demo/` — a reference server implementation behind
the `authsidecar_demo` build tag. It reuses the lark-cli credential
pipeline for local development; production integrators are expected
to replace the credential layer with their own secrets source.
- Update all internal imports from `internal/sidecar` to `sidecar`.
Rationale:
- Each integrator has different secrets management / HA / multi-tenant
requirements, so a one-size-fits-all server doesn't belong in the
shipped CLI.
- Keeping the client in-tree guarantees all sandbox-side code stays
protocol-compatible without a second repo to sync.
- The public `sidecar/` package pins the wire protocol as a stable
contract third-party servers must conform to.
Build matrix after this change:
- `go build` → standard CLI, no sidecar code
- `go build -tags authsidecar` → CLI + sidecar client
- `go build -tags authsidecar_demo \
./sidecar/server-demo/` → reference server binary
No production users are affected today because the server was not yet
released; existing sidecar-client users are unchanged.
* feat(sidecar): close 5 pre-release security gaps
- Server: enforce https-only target (no path/query/userinfo), pin
forwardURL to https:// — blocks cleartext token leak
- Protocol v1: canonical now covers version/identity/auth-header,
blocks identity-flip replay within drift window
- Client: ValidateProxyAddr requires loopback or same-host alias,
rejects userinfo and https (interceptor is http-only); cross-machine
is out of scope
- Build: non-authsidecar builds exit(2) when AUTH_PROXY is set,
preventing silent fallback to env credentials
- Demo: whitelist auth-header to Authorization / X-Lark-MCP-{UAT,TAT},
blocks token injection into Cookie / UA / X-Forwarded-For exfil paths
fix: address coderabbit review comments on role-config docs - Update `allow_edit` field description to reflect conditional default: `true` when table perm is `edit`, `false` for `read_only` or explicit restriction - Move `record_operations.delete` out of "默认关闭项" into new "默认开启项（条件性）" section to accurately reflect it is default-included when `perm = edit` - Add `view_rule.allow_edit` to "默认开启项（条件性）" section with same logic Co-authored-by: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
domain/ccm
* fix(doc): preserve round-trip formatting in fetch output - trim leading spaces inside bold and italic emphasis exported by docs +fetch - normalize nested list indentation to avoid flattening and literal text on re-import - add regression tests for emphasis spacing and nested list indentation * fix(doc): avoid false positives in markdown spacing fixes - keep literal * x * and ** x ** text unchanged - only normalize indented nested list markers when a parent list item exists - add regression coverage for both CodeRabbit findings * fix(doc): 修正嵌套列表缩进的空行误判 - 遇到空行时停止向上查找父级列表项，避免把 loose list sibling 误改成嵌套列表 - 避免把列表项中的四空格缩进代码块误改成 tab 缩进列表项 - 补充两个回归测试，并更新 fixBoldSpacing 注释使其与当前实现一致 * fix(doc): 修复 Markdown emphasis 空格回写 - 将 fixBoldSpacingLine 改为按星号 run 扫描，修复 ** hello **、* hello * 和同一行多个 italic span 的空格清理 - 保留 inline code、heading 和 *** hello** 这类近邻字面量，避免误改 emphasis nesting
Adds 5 invariant-level tests on top of larksuite#469's transforms: - TestFixExportedMarkdownIdempotent — f(f(x)) == f(x) across rich fixtures (kitchen sink, CJK, nested containers). Protects the core round-trip promise from future transform interactions that rewrite their own output. - TestFixExportedMarkdownPreservesFencedCodeByteForByte — packs every pipeline-touching shape into a fence and asserts byte-identical output. Code samples must never be silently rewritten by a formatting pass. - TestFixExportedMarkdownPreservesCRLF — CRLF input preserves line endings AND still triggers transforms. Windows-authored markdown should not be silently LF-normalized. - TestFixExportedMarkdownTransformInteractions — composition regressions: nested-list + trailing-space bold, text→list transition, callout containing list with emphasis, heading vs paragraph bold. - TestNormalizeNestedListIndentationDocumentedSkips — locks in the deliberate no-op branches (odd-space indent, blank-line loose-list sibling, 4-space indented code block, parentless two-space) as an explicit spec so future heuristic tweaks surface in the test diff. All transforms, fixtures, and expectations are derived from the head of PR larksuite#469. No production code changes.
fangshuyu-768
force-pushed
the
test/markdown-fix-hardening
branch
from
9a983f3 to
5ecd444
Compare
github-actions
Bot
added
domain/base
Author
|
PR larksuite#469 merged to |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds 5 invariant-level tests on top of larksuite#469 to lock in the properties the PR is trying to establish. No production code changes — pure test coverage. Targeting
fix/docs-fetch-roundtrip-formattingso the tests land together with the implementation they exercise.What's added
TestFixExportedMarkdownIdempotentf(f(x)) == f(x)across kitchen-sink / CJK / nested-container fixtures — the round-trip promiseTestFixExportedMarkdownPreservesFencedCodeByteForByteTestFixExportedMarkdownPreservesCRLFTestFixExportedMarkdownTransformInteractionsTestNormalizeNestedListIndentationDocumentedSkipsWhy
These shapes aren't covered by the existing table-driven tests in
markdown_fix_test.go. Each test targets a structural invariant rather than a specific fixture:strings.Containsassertions: diffs byte-by-byte.Fix*functions are well-tested in isolation; composition wasn't.Test plan
go test ./shortcuts/doc/...— all tests pass (previous + 13 new sub-tests)gofmtcleangolangci-lint run— no new issues introduced by this fileNote
Targeting
fix/docs-fetch-roundtrip-formattingrather thanmainso the diff stays scoped to the added test file (mainhas moved on since larksuite#469's base commit, so a main-targeted PR would sweep in unrelated changes). If the preference is to land this as a separate follow-up after larksuite#469 merges, happy to rebase and retarget — just let me know.