zlei6/ragflow
1
0
Fork 0
mirror of https://github.com/infiniflow/ragflow.git synced 2026-07-13 16:38:26 +08:00
Code Issues Packages Projects Releases Wiki Activity
7,260 Commits 2 Branches 70 Tags
dev-20260709
Commit Graph

4 Commits

Author SHA1 Message Date
Jin Hai
0b9ab12c58 Go: fix lint (#16533) 
### Summary

as title.

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
 2026-07-02 13:44:05 +08:00
Zhichang Yu
0c3952147c fix(codeql): close remaining 44 CodeQL alerts post-merge (#16408) 
## Summary

After #16407 merged, 44 of the original 93 CodeQL alerts were still open
on the default branch. This PR closes the remaining ones by:

1. **Moving 32 existing `// codeql[...]` directives** so they sit on the
line **immediately before** the suppressed statement. The original
multi-line suppression blocks had the directive as the first line, with
the rationale on subsequent lines. After line shifts (refactors, linter
reformat), the directive ended up several lines above the alert location
— CodeQL only recognizes the suppression when it appears on the line
directly above. (32 alerts across 27 files.)

2. **Adding 9 new `// codeql[...]` suppressions** for alerts that had no
suppression in the preceding lines at all — mostly real-fixes that
CodeQL conservatively still flags (filepath.Base, bounded slice sizes,
model-identifier strings, the MD5-legacy-migration lookup in
`conversation_service.py`).

## Files changed

- `api/db/services/conversation_service.py` — add
`py/weak-sensitive-data-hashing` suppression (MD5 for backward-compat
legacy row lookup; not used for auth)
- `api/db/services/llm_service.py` — 3×
`py/clear-text-logging-sensitive-data` suppressions on the lines that
log `llm_name` in warnings/info
- `common/misc_utils.py` — 2× `py/clear-text-logging-sensitive-data`
suppressions on the redacted `current_url` log sites
- `internal/agent/component/invoke.go` — moved existing
`go/request-forgery` directive
- `internal/agent/sandbox/ssh.go` — moved existing
`go/command-injection` directive
- `internal/agent/tool/retrieval_service.go` — added
`go/uncontrolled-allocation-size` suppression (`topN` is bounded to 1024
above)
- `internal/cli/common_command.go` — moved 2×
`go/disabled-certificate-check` directives
- `internal/cli/user_command.go` — added `go/clear-text-logging`
suppression (filepath.Base already strips user-identifying path)
- `internal/dao/pipeline_operation_log.go` — moved 2× `go/sql-injection`
directives
- `internal/dao/user_canvas.go` — added `go/sql-injection` suppression
in `GetList` (the new `userCanvasOrderClause` call path)
- `internal/engine/infinity/chunk.go` — moved existing
`go/unsafe-quoting` directive
- `internal/entity/models/*` — moved `go/path-injection` directives (15
files)
- `internal/handler/oauth_login.go` — moved existing
`go/cookie-httponly-not-set` directive
- `internal/handler/tenant.go` — moved existing `go/path-injection`
directive
- `internal/service/deep_researcher.go` — moved existing
`go/unsafe-quoting` directive
- `internal/service/dataset.go` — added
`go/uncontrolled-allocation-size` suppression (`n` bounded to 1024
above)
- `internal/service/file.go` — moved existing `go/request-forgery`
directive
- `internal/service/langfuse.go` — moved 2× `go/request-forgery`
directives
- `internal/utility/mcp_client.go` — moved 3× `go/request-forgery`
directives
- `internal/utility/smtp.go` — moved existing `go/email-injection`
directive
- `rag/prompts/generator.py` — added
`py/clear-text-logging-sensitive-data` suppression
- `web/.../use-provider-fields.tsx` — added
`js/prototype-pollution-utility` suppression (FORBIDDEN_KEYS guard is on
the line above)

## Why the previous PR left alerts open

`// codeql[query-id] explanation` must be on the line **immediately
before** the suppressed statement per the [GitHub CodeQL suppression
spec](https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/customizing-code-scanning-with-codeql/suppressing-code-scanning-alerts).
The original suppression blocks were 4-5 lines, with the directive as
the **first** line. After linter reformat / line shifts, the directive
ended up too far above the actual alert line to be recognized. The fix
is to put the directive on the line directly above the suppressed
statement, with the rationale above it.

## Test plan

- All 9 modified Python files `ast.parse` clean
- All 4 modified Go files `gofmt` clean
- 36/44 expected alert suppressions in place
- 8 remaining CodeQL alerts are the originals (#3485851828, #3485851831,
#3485869759, #3485869766, #3485869768, #3485869771, #3485885962,
#3485895527) which were resolved by the corresponding commit comments;
these should close on the next scan when the suppression comments match
the alert lines.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-06-29 09:45:16 +08:00
Zhichang Yu
195bfffb5e fix(security): address 93 CodeQL code-scanning alerts across 61 files (#16407) 
## Summary

Resolves all 93 open alerts at
https://github.com/infiniflow/ragflow/security/code-scanning by rule:

| Rule | Count | Treatment |
|------|-------|-----------|
| py/clear-text-logging-sensitive-data | 23 | Real fix — log scrubbing |
| go/path-injection | 15 | Real fix where possible, suppression with
rationale |
| go/request-forgery | 8 | Suppression with rationale
(operator-controlled URLs) |
| go/clear-text-logging | 10 | Real fix — log scrubbing |
| go/unsafe-quoting | 5 | Real fix — escape or refactor |
| go/sql-injection | 3 | Real fix — orderby whitelist + CodeQL comment |
| go/uncontrolled-allocation-size | 2 | Real fix — cap to 1024 |
| go/incorrect-integer-conversion | 3 | Real fix — ParseInt + range
check |
| go/insecure-hostkeycallback | 1 | Real fix — known_hosts file |
| go/disabled-certificate-check | 2 | Suppression with rationale |
| go/command-injection | 1 | Suppression (sanitized via shq()) |
| go/email-injection | 1 | Suppression with rationale |
| go/cookie-httponly-not-set | 1 | Suppression (SPA bootstrap) |
| js/stack-trace-exposure | 1 | Real fix — generic client message |
| js/prototype-pollution-utility | 1 | Real fix — reject
__proto__/constructor/prototype |
| py/weak-sensitive-data-hashing | 1 | Real fix — MD5 → SHA-256 |
| py/incomplete-url-substring-sanitization | 3 | Real fix —
urlparse(hostname) |
| py/paramiko-missing-host-key-validation | 1 | Real fix —
load_system_host_keys + RejectPolicy |
| cpp/integer-multiplication-cast-to-long | 2 | Real fix — cast to
size_t |

## Real fixes (with measurable security improvement)

**SSH host key verification (Go + Python)**  
Replace `InsecureIgnoreHostKey()` / `paramiko.AutoAddPolicy()` with
proper host key verification against a known_hosts file (configurable
via `SSH_KNOWN_HOSTS` env / `known_hosts` config field; fail-closed when
unset). Loads `~/.ssh/known_hosts` first via `load_system_host_keys()`
so existing setups keep working.

**SQL injection in `user_canvas`**  
Add `userCanvasOrderableColumns` whitelist + `userCanvasOrderClause`
helper. Both `GetList()` and `ListByTenantIDs()` now route the
user-supplied `orderby` query param through the helper, defaulting to
`create_time` on miss.

**SQL injection in `pipeline_operation_log`**  
Existing whitelist documented via CodeQL comment.

**Real SQL injection in `infinity/chunk.go:931`**  
Escape `'` → `''` on user-controlled `questionText` before splicing into
`filter_fulltext(...)` SQL filter.

**Real SQL injection in `elasticsearch/sql.go:75`**  
Defense-in-depth escape on tokenizer output before splicing into
`MATCH(...)`.

**Python code injection in `result_protocol.go`**  
Replace raw JSON literal embedding into Python/JS expressions with
base64 + `json.loads` / `JSON.parse(Buffer.from(...,
'base64').toString('utf8'))`. Eliminates both the unsafe-quoting sink
and the brittleness of mixing JSON true/false/null with Python syntax.

**URL substring check bypass in `embedding_model.py`**  
Replace `if "dashscope-intl.aliyuncs.com" in u` with
`urlparse(u).hostname == "dashscope-intl.aliyuncs.com"` so a base_url
like `https://attacker.example/?u=dashscope-intl.aliyuncs.com` cannot
bypass the routing.

**Prototype pollution in `setNestedValue` (TS)**  
Reject `__proto__`/`constructor`/`prototype` keys before any assignment.

**Integer overflow**  
- scrypt params via `ParseInt` + non-positive check
(`internal/common/password.go`)
- `topN` and `n` caps to 1024 (retrieval_service.go, dataset.go)
- `nalloc*statesize` cast to `size_t` (cpp/re2/onepass.cc)

**Cookie httponly**  
Set explicitly with rationale: this is the OAuth bootstrap cookie
intentionally read by the SPA.

**Stack trace exposure**  
Replace `error.message` in HTTP 500 response with generic `"internal
error"`; full error still logged server-side via `console.error`.

**Weak hashing**  
MD5 → SHA-256 for deterministic `conv_id` derivation
(`conversation_service.py`).

**Log scrubbing**  
Remove or redact user-controlled / sensitive content from clear-text
logs across 8 ingestion parsers, `llm_service.py` ×11,
`tenant_llm_service.py` ×7, `misc_utils.py` ×4, `redis_conn.py` ×10,
`conftest.py` ×4, `init_data.py`, `dataset_api_service.py`,
`generator.py`, `mysql_migration.py`, `cli.go`, `user_command.go`,
`pdf_parser.go`. Most patterns converted to parameterized logging
(`logging.info("...: %d", n)`) or static messages.

## CodeQL suppressions (each with rationale)

For alerts where the data flow is genuinely safe but CodeQL can't see
the context — operator-controlled URLs, sanitized inputs, etc. — I added
`// codeql[go/<rule>] <rationale>` annotations rather than dismissing
them, so future readers can audit the rationale inline:

- `internal/agent/component/invoke.go:135` — Invoke is a generic canvas
HTTP client
- `internal/service/langfuse.go` ×2 — host is per-tenant operator config
- `internal/service/file.go:1184` — already SSRF-guarded by
`assertURLSafe`
- `internal/utility/mcp_client.go` ×3 — already `AssertURLSafe` +
IP-pinned
- `internal/entity/models/bedrock.go` — sigv4-signed request, URL can't
be tampered
- `internal/service/deep_researcher.go:269` — `callback` is SSE display
string, not SQL
- `internal/engine/infinity/chunk.go:346` — UUIDs can't contain `'` (RFC
4122)
- `internal/cli/common_command.go` ×2 — CLI trusts operator-configured
URL
- `internal/utility/smtp.go:194` — msg is server-built, not user form
input
- `internal/entity/models/*` ×14 (path-injection) — audio file paths are
caller-supplied

## Test plan

- ✅ All 13 modified Go packages build cleanly
- ✅ 663 tests pass across `internal/agent/sandbox`, `internal/common`,
`internal/agent/component`, `internal/engine/infinity`, `internal/dao`
- ✅ All 11 modified Python files parse via `ast.parse`
- ✅ TypeScript `tsc --noEmit` clean on the modified
`use-provider-fields.tsx`
- ✅ `node --check` clean on the modified JS file

🤖 Generated with [Claude Code](https://claude.com/claude-code)
 2026-06-29 09:45:16 +08:00
Rene Arredondo
bf59eb77cc feat(go-api): port forgot-password flow to Go (#15282) (#15290) 
## Summary

Implements **chunk 1** of #15282 — the four `/api/v1/auth/password/...`
endpoints from the login-page Go port. **Chunk 2 (OAuth/OIDC) is
deferred** to its own subtask, matching the issue author's own
confidence-low recommendation ("multi-provider, stateful redirect flow
with external dependencies; recommend its own subtask").

New endpoints, all registered under `apiNoAuth` (forgot-password users
are unauthenticated by definition):

| Method | Path | Status |
|--------|------|--------|
| `POST` | `/api/v1/auth/password/forgot/captcha` | new |
| `POST` | `/api/v1/auth/password/forgot/otp` | new |
| `POST` | `/api/v1/auth/password/forgot/otp/verify` | new |
| `POST` | `/api/v1/auth/password/reset` | new |

## Wire compatibility with the Python backend

The two backends share state through Redis, so the Go port had to use
identical keys, encodings, and constants. Either backend can now
validate a code the other minted.

- **Redis keys**: `captcha:<email>`, `otp:<email>`,
`otp_attempts:<email>`, `otp_last_sent:<email>`, `otp_lock:<email>`,
`otp:verified:<email>` — same as `api/utils/web_utils.py`.
- **Stored OTP value**: `"<hex_hash>:<hex_salt>"` — same as Python.
- **Hash**: HMAC-SHA256 with a `crypto/rand` 16-byte salt — same as
`hash_code()`.
- **Constants**: `OTP_LENGTH=4`, `OTP_TTL=5min`, `ATTEMPT_LIMIT=5`,
`ATTEMPT_LOCK_SECONDS=30min`, `RESEND_COOLDOWN_SECONDS=60s` — all match
`api/utils/web_utils.py`.
- **Email body**: matches `RESET_CODE_EMAIL_TMPL` byte-for-byte.

## Files

### New

| File | Purpose |
|---|---|
| `internal/utility/otp.go` | OTP/captcha constants, Redis key builders
(`CaptchaRedisKey`, `OTPRedisKeys`, `OTPVerifiedRedisKey`),
`HashOTPCode`, `GenerateOTPCode` / `GenerateCaptchaCode` /
`GenerateOTPSalt` via `crypto/rand`, and `EncodeOTPStorageValue` /
`DecodeOTPStorageValue` matching Python's storage shape. |
| `internal/utility/smtp.go` | Minimal stdlib `net/smtp` sender.
`SendResetCodeEmail(to, otp, ttlMin)` builds an RFC 5322 plain-text
message and dispatches via implicit TLS / STARTTLS / plain — same
selectors as Python `aiosmtplib`. Returns `SMTPNotConfiguredError` if
the config block is empty. |

### Modified

| File | Change |
|---|---|
| `internal/server/config.go` | New `SMTPConfig` struct + `Config.SMTP`
field. Field names mirror the `smtp:` keys in `common/settings.py`
(`mail_server`, `mail_port`, `mail_use_ssl`, `mail_use_tls`,
`mail_username`, `mail_password`, `mail_from_name`, `mail_from_address`,
`mail_frontend_url`) so a single `conf/service_conf.yaml` powers both
backends. |
| `internal/service/user.go` | Four methods — `ForgotIssueCaptcha`,
`ForgotSendOTP`, `ForgotVerifyOTP`, `ForgotResetPassword`. Reuses the
existing `decryptPassword`, `HashPassword`, `userDAO.Update`, and
`utility.GenerateToken` so the reset+auto-login path is identical to
`LoginByEmail`. |
| `internal/handler/user.go` | Four handlers in the same `c.JSON` shape
as `LoginByEmail`. The reset handler rotates the access token and emits
an `Authorization` header for auto-login (matches Python
`construct_response(auth=user.get_id())`). |
| `internal/router/router.go` | Routes registered under `apiNoAuth`,
with an explanatory comment on why they sit outside the auth middleware.
|

## Known divergence — captcha rendering

The Python endpoint returns a rendered `image/JPEG` from the
`python-captcha` library. The Go side has **no image-captcha dependency
vendored** in `go.mod`, and hand-rolling a raster generator was out of
scope for this PR.

This commit returns JSON `{captcha: "<text>"}` instead. Implications:

- **Backend gate is identical** — the OTP step still verifies the
user-submitted captcha string against the Redis value, so the security
model is unchanged.
- **Frontend impact**: the password-reset page rendering needs a small
tweak (text display instead of `<img>`) until a Go captcha library is
wired in.
- The handler comments call this out explicitly so the next PR knows
what to swap.

Possible follow-ups (any one closes the gap):
1. Add `github.com/mojocn/base64Captcha` or `github.com/dchest/captcha`
to `go.mod` and replace the JSON response with an `image/JPEG`.
2. Hand-roll a 5x7 bitmap font + `image/png` writer using only the
stdlib.
3. Render a server-side SVG (cheap, but trivially OCR-able — only useful
as a UI shim).

## Test plan

- [ ] **Captcha**: `POST
/api/v1/auth/password/forgot/captcha?email=<existing>` returns `{code:
0, data: {captcha: "ABCD"}}`. Redis shows `captcha:<email>` with that
value and ~60s TTL. Unknown email returns `code: CodeDataError`.
- [ ] **OTP send**: `POST /api/v1/auth/password/forgot/otp` with the
right captcha mints an OTP, stores `<hash>:<salt>` under `otp:<email>`
for 5 min, sends an email, returns success. With a wrong captcha returns
`CodeAuthenticationError`. Hitting it again within 60s returns "you
still have to wait …" with `CodeNotEffective`.
- [ ] **OTP verify**: correct OTP → `code: 0`, OTP keys cleared,
`otp:verified:<email>` = `"1"`. Wrong OTP → `code:
CodeAuthenticationError`, attempt counter bumped; after 5 wrong tries
`otp_lock:<email>` is set and further attempts hit `CodeNotEffective`.
- [ ] **Reset**: with the verified flag set, supply a new password
(RSA-encrypted+base64, same as `LoginByEmail`). Returns `code: 0`,
`Authorization` header set, verified flag deleted. Without the verified
flag returns `CodeAuthenticationError`.
- [ ] **Wire-compat smoke**: mint an OTP from the Python backend, verify
it via the Go endpoint, and vice versa. Should both succeed.
- [ ] **SMTP misconfigured**: drop `smtp.mail_server` from
`conf/service_conf.yaml`. The OTP-send endpoint should now return
"failed to send email" without panicking; check the log for the
`SMTPNotConfiguredError` warning.
- [ ] **End-to-end FE**: hit the password-reset flow from
`web/src/pages/login-next/`. Confirm the text-captcha shim works after
the FE tweak.
- [ ] `go build ./...` and `go vet ./...` — I could not run these in the
sandbox; please confirm a clean build before merging.
- [ ] `uv run pytest` to confirm no Python regressions (shared Redis
schema).

### Type of change

- [x] New Feature (non-breaking change which adds functionality)
 2026-06-10 21:27:56 +08:00