chore: 第三方登录合并后的遗留修复 - #31
Merged
Merged
Conversation
A cross-check turned up two places that still described a superseded layout. API文档 §4.2 and the OpenAPI github bind operation both offered "register a second OAuth App" as an alternative. That does not work: Bind() exchanges the code with OAUTH_GITHUB_CLIENT_ID/SECRET, so a code issued by a different App is rejected. Both now say so and point at the shared-parent-path approach instead, with the production and local registration values spelled out. PRD §4.8 listed the bind endpoints with only ?code=xxx and never said where that code comes from — the same omission that made the whole flow unimplementable before. It now carries redirect_uri and states that the bind code does not pass through this service's callback at all: the frontend receives it and forwards it, which is why the token exchange has to repeat the frontend's own callback. All four documents plus .env.example now reference the Caddy runbook for the configuration detail rather than each restating it, so the next change to the callback layout has one place to edit. The Lark bind description was already correct and is unchanged: Feishu accepts several redirect URLs per app, so it has no shared-parent constraint.
Verified against a real password-protected Redis container rather than by
inspection; two of the findings below only showed up once actually run.
The container names were the pre-compose ones (sastlink-postgres,
sastlink-redis) while compose creates sastlink-compose-{postgres,redis}. This one
does fail loudly — the preflight check reports "容器 xxx 未运行" and exits — so it
was an obstacle rather than a silent defect. Both names are now overridable and
default to what compose creates.
redis-cli was called without -a, but compose starts Redis with --requirepass.
Measured: every call returns "NOAUTH Authentication required." All calls now go
through one redis_cli helper that passes the password only when there is one; the
${VAR:+-a "$VAR"} form was checked to keep a password containing spaces as a
single argument and to omit -a entirely when unset.
The verification-code read was unguarded, and this is the defect that mattered.
redis-cli writes NOAUTH to stdout and still exits 0, so the existing
`|| echo ""` fallback never fired and the `-z` check passed: CODE became the
literal string "NOAUTH Authentication required." and was submitted as a
verification code, failing later and further from the cause. It is now validated
as six digits, and the error message reports what Redis actually returned.
REDIS_PREFIX and the psql credentials were resolved before `source .env` on line
65, so REDIS_PREFIX always took its "sastlink" default. With REDIS_KEY_PREFIX set
the script wrote the code under a key the service does not read, and psql used a
hardcoded user instead of DB_USER. They now resolve after the env file loads.
PG_PORT and REDIS_PORT are dropped rather than corrected: psql and redis-cli both
run through docker exec inside the container, so no host port is involved and
neither variable was ever read.
The runtime stage inherited root from the alpine base. Both binaries are static, take their configuration from the environment, and write nothing to the filesystem, so root buys nothing — and 8080 is above 1024, so binding it needs no capability either. Verified by building the image and running id inside it: sastlink, uid 10001. A container escape or an RCE in a dependency now lands as an unprivileged user with no write access to anything in the image, rather than as root.
CLAUDE.md described docker-compose.yml as running a prebuilt image against external postgres and redis networks, and explicitly said it "does not build an image from this repository". Compose now does the opposite: it provisions both stores, builds from the repository Dockerfile, and gates the API behind a one-shot migration. Both the commands section and the deployment notes stated the old arrangement, so a reader following either would have looked for networks that no longer exist. The test command in all three files was unquoted. PowerShell splits -a=b at the "=", so `-coverprofile=coverage.out` arrives as two arguments and go test treats ".out" as a package path, failing with "no required module provides package .out" before a single test runs. This cost time twice in one session, once on -coverprofile and once on `go tool cover -func=`, so the caveat now sits next to the command rather than waiting to be rediscovered. The CI entry keeps the unquoted form and notes that its runner is bash, where the problem does not exist. Also indexes the local flow script in the source-of-truth list, noting that its container defaults track compose and that the third-party legs need a browser.
s3loy
approved these changes
Jul 31, 2026
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
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.
PR #30 合并后发现的四项遗留问题。
本地测试脚本(
scripts/local-oauth-complete-flow.sh)起了一个带
--requirepass的真实 Redis 容器实测,而非只读代码。redis-cli把NOAUTH写到 stdout 且退出码为 0,所以既有的|| echo ""兜底不触发、-z检查也通过——CODE会变成字符串"NOAUTH Authentication required."并被当作验证码提交,在更远处失败。现已校验为六位数字,错误信息带上 Redis 实际返回值。redis-cli缺-a,而 compose 用--requirepass起 Redis,每条命令都 NOAUTH。统一走一个 helper,仅在有密码时传-a。sastlink-postgresvssastlink-compose-postgres)。这项会明确报错退出,属于障碍而非静默缺陷。REDIS_PREFIX与 psql 凭据在source .env之前取值,所以设了REDIS_KEY_PREFIX时脚本写的键与服务读的键不是同一个。PG_PORT/REDIS_PORT:psql与redis-cli都通过docker exec在容器内执行,不走宿主端口,这两个变量从未被引用。Dockerfile
运行阶段从 alpine 继承了 root。两个二进制都是静态的、配置从环境读、不写文件系统,8080 也无需特权。实测镜像内运行身份为
sastlink/ uid 10001。文档
CLAUDE.md关于 compose 的描述与现状相反——它写着「不从本仓库构建镜像」「外部 postgres/redis 网络」,而 compose 现在自带两个存储、从 Dockerfile 构建、API 等 migration 成功退出后才启动。命令段与部署段两处都是旧的。=处切开-coverprofile=coverage.out,go test把.out当包路径,测试一条都没跑就失败。本次会话踩了两次(-coverprofile=与go tool cover -func=),现记在命令旁边。CI 那条保留原形并注明其 runner 是 bash。Bind()用OAUTH_GITHUB_CLIENT_ID/SECRET交换 code,另一个 App 签发的 code 会被拒。PRD §4.8 的端点表也缺redirect_uri且未说明 code 从何而来。验证
go build ./...、docker compose config、docker build、bash -n、openapi YAML 校验均通过。正则守卫逐例验证:123456接受,NOAUTH 文本 / 空值 / 5 位 / 7 位 / 含字母全部拒绝。未改动 Go 代码。