fix(audit): pass 1 — overflow guard, prefix-match, dead guards, comments

- Config.cpp parseDurationNs: reject uint64 overflow on segment
  multiply and running-sum; a typo like 1000000000y would otherwise
  wrap to a small bogus value the caller treats as a real duration.
- Config.cpp hasCustomShader: rebuild on top of diskValue so the
  prefix-matched scan no longer mistakes a future
  `custom-shader-animation = …` key for our repeating-path entry,
  and so a trailing `custom-shader =` correctly clears earlier
  assignments (libghostty repeating-key semantics).
- Config.h: get<>() now static_asserts N > 1 to catch empty-literal
  fat-finger calls; durationNs docstring corrected — non-optional
  Duration keys actually surface through ghostty_config_get (in ms),
  so the wrapper is for `?Duration` only.
- GhosttySurface scrollbarAllowed/flashScrollbar/paintResizeOverlay:
  drop the m_owner null guards left over from the pre-namespace API.
  config::get/string are null-safe (return false/empty when handle()
  is null) so the guards were dead code with the same outcome.
- ChromeActions COLOR_CHANGE: setColorScheme is process-global, so
  the winp gate silently dropped chrome flips for *other* live
  windows when the originating window closed mid-dispatch. Drop
  the capture entirely.
- SystemActions PROGRESS_REPORT: trim the structurally-impossible
  type-confusion warning; the typed config::boolean wrapper hides
  the libghostty out-pointer entirely.
- GhosttyApp.cpp: re-add the explicit <QByteArray> include — it is
  used directly (clipboard text/content) and was surviving only via
  transitive headers.

Co-Authored-By: claude-flow <ruv@ruv.net>
pull/12846/head
ntomsic 2026-05-23 14:25:59 -05:00
parent bf847b1608
commit 7a01fe00b9
6 changed files with 43 additions and 38 deletions

View File

@ -242,7 +242,8 @@ void GhosttySurface::layoutScrollbar() {
// `scrollbar = never` in the config hides the scrollbar unconditionally;
// `system` (the default) shows it whenever there is scrollback.
bool GhosttySurface::scrollbarAllowed() const {
if (!m_owner || !m_owner->config()) return true;
// config::get is null-safe (returns false when handle() is null),
// so we only need the "could not read" → default-to-showing path.
const char *value = nullptr;
if (config::get(&value, "scrollbar") && value)
return qstrcmp(value, "never") != 0;
@ -271,7 +272,7 @@ void GhosttySurface::flashScrollbar() {
if (!m_scrollbar || !scrollbarAllowed()) return;
// Handle colour: light on a dark terminal, dark on a light one.
ghostty_config_color_s bg{};
if (m_owner && config::get(&bg, "background")) {
if (config::get(&bg, "background")) {
const double luma = 0.299 * bg.r + 0.587 * bg.g + 0.114 * bg.b;
m_scrollbar->setHandleColor(luma < 128.0 ? QColor(235, 235, 235)
: QColor(45, 45, 45));
@ -583,8 +584,7 @@ void GhosttySurface::paintResizeOverlay(QPainter &painter) {
const qreal boxH = ts.height() + 2 * padY;
// resize-overlay-position: center / {top,bottom}-{left,center,right}.
const QString pos =
m_owner ? config::string("resize-overlay-position") : QString();
const QString pos = config::string("resize-overlay-position");
const qreal m = 8;
qreal x = (width() - boxW) / 2;
qreal y = (height() - boxH) / 2;

View File

@ -87,10 +87,12 @@ bool handleChrome(const Context &ctx, const ghostty_action_s &action) {
if (action.action.color_change.kind ==
GHOSTTY_ACTION_COLOR_KIND_BACKGROUND) {
const ghostty_action_color_change_s c = action.action.color_change;
post(qApp, [winp, c]() {
// Bail if the originating window was closed between the
// action posting and this slot — no chrome to flip.
if (!winp) return;
// No window capture: setColorScheme and config::string are
// both process-scoped, so the originating window's lifetime
// doesn't affect this slot's correctness — and gating on it
// would silently drop chrome flips for the *other* windows
// that are still alive.
post(qApp, [c]() {
if (config::string("window-theme") != QLatin1String("ghostty"))
return;
const double luma = 0.299 * c.r + 0.587 * c.g + 0.114 * c.b;

View File

@ -188,12 +188,8 @@ bool handleSystem(const Context &ctx, const ghostty_action_s &action) {
case GHOSTTY_ACTION_PROGRESS_REPORT: {
// Honor `progress-style`: when false, OSC 9;4 progress
// sequences are silently ignored (no taskbar entry). It is a
// *bool* in Config.zig — it MUST be read with config::boolean.
// config::string would hand ghostty_config_get a `const char**`;
// the 1-byte bool write leaves a `0x1` pointer that
// QString::fromUtf8 then dereferences and crashes on (e.g.
// when Claude emits progress).
// sequences are silently ignored (no taskbar entry). The gate
// is process-wide (a config setting, not a per-window setting).
if (!config::boolean("progress-style", true)) return true;
const ghostty_action_progress_report_s p = action.action.progress_report;
const ghostty_action_progress_report_state_e state = p.state;

View File

@ -3,6 +3,7 @@
#include <cstdio>
#include <QApplication>
#include <QByteArray>
#include <QClipboard>
#include <QCoreApplication>
#include <QEvent>

View File

@ -1,5 +1,7 @@
#include "Config.h"
#include <climits>
#include <QByteArray>
#include <QChar>
#include <QDir>
@ -51,11 +53,13 @@ QString diskValue(const char *key) {
// concatenated `<n><unit>` segments per Config.zig's Duration.parseCLI:
// y w d h m s ms µs us ns
// Each segment is added to the total. Returns the supplied fallback
// when parsing fails or the input is empty.
// when parsing fails, when the input is empty, or when the running
// total would overflow uint64 (Config.zig rejects this; we mirror).
static uint64_t parseDurationNs(const QString &s, uint64_t fallback) {
if (s.isEmpty()) return fallback;
// Order matters: longer units first so `ms` is matched before `s`,
// `us` before `s`, etc. Mirrors Config.zig's units array.
// kUnits mirrors Config.zig's units array; the longest-prefix match
// at the matching site below makes table order semantically
// irrelevant (kept aligned with Zig for diffability).
static constexpr struct { const char *name; uint64_t factor; } kUnits[] = {
{"ns", 1ULL},
{"us", 1000ULL},
@ -99,7 +103,13 @@ static uint64_t parseDurationNs(const QString &s, uint64_t fallback) {
}
}
if (unitLen == 0) return fallback;
total += value * factor;
// Reject overflow on multiply or running-sum: a typo like
// `1000000000y` would otherwise wrap into a small bogus value
// that callers treat as a real (tiny) duration.
if (factor && value > ULLONG_MAX / factor) return fallback;
const uint64_t segment = value * factor;
if (segment > ULLONG_MAX - total) return fallback;
total += segment;
i += unitLen;
anyMatched = true;
}
@ -125,22 +135,12 @@ QString expandedPath(const char *key) {
bool hasCustomShader() {
// libghostty does not expose this through ghostty_config_get
// (`custom-shader` is a repeatable path), so scan the primary
// config file directly.
QString dir = qEnvironmentVariable("XDG_CONFIG_HOME");
if (dir.isEmpty()) dir = QDir::homePath() + QStringLiteral("/.config");
QFile f(dir + QStringLiteral("/ghostty/config"));
if (!f.open(QIODevice::ReadOnly | QIODevice::Text)) return false;
while (!f.atEnd()) {
const QByteArray line = f.readLine().trimmed();
if (!line.startsWith("custom-shader")) continue;
// Require a non-empty value: `custom-shader =` alone clears it.
const int eq = line.indexOf('=');
if (eq >= 0 && !line.mid(eq + 1).trimmed().isEmpty()) return true;
}
return false;
// (`custom-shader` is a repeatable path), so scan the on-disk
// config text. diskValue does the exact-key match (so
// `custom-shader-animation = …` is not mistaken for our key) and
// last-write-wins (so `custom-shader =` clears any earlier
// assignment, matching libghostty's repeating-key semantics).
return !diskValue("custom-shader").isEmpty();
}
} // namespace config

View File

@ -32,10 +32,15 @@ QString string(const char *key);
// strict bools, NOT packed bitfields — see bitfield<>() for those.
bool boolean(const char *key, bool fallback);
// Parse a duration config key as nanoseconds. Always reads through
// the disk-fallback (configDiskValue) because libghostty's
// ghostty_config_get rejects Duration types (non-extern non-packed
// struct). Returns `fallbackNs` on parse failure or absent key.
// Parse a duration config key as nanoseconds via the on-disk
// fallback. Use this for `?Duration` (optional) keys: c_get.zig
// returns false for a null optional, so the disk text is the only
// way to recover the configured value. Non-optional `Duration` keys
// surface through ghostty_config_get directly (it returns the value
// in *milliseconds*, per Duration.cval()) and should use config::get
// with `unsigned long long` and a manual ms→ns multiplication, NOT
// this wrapper, to avoid a redundant disk re-scan on every read.
// Returns `fallbackNs` on parse failure or absent key.
uint64_t durationNs(const char *key, uint64_t fallbackNs);
// Scan the user's primary on-disk config file for `key = value`
@ -77,6 +82,7 @@ QString expandedPath(const char *key);
// fails.
template <typename T, size_t N>
inline bool get(T *out, const char (&key)[N]) {
static_assert(N > 1, "config::get requires a non-empty key literal");
ghostty_config_t cfg = handle();
return cfg && ghostty_config_get(cfg, out, key, N - 1);
}