Aikido

Grafanaのエンジニアリング・チームから学んだ10のコード品質ルール

はじめに

Grafanaは最も人気のあるオープンソースのobservabilityプラットフォームの1つで、70k以上のGitHubスターと何千人もの貢献者が毎日それを改善しています。3,000以上のオープンイシューと何百ものプルリクエストが常に動いており、コードベースをクリーンで一貫性のあるものに保つことは本当に難しいことです。

そのコードベースを研究することで、チームが迅速に動きながら高い品質を維持するための不文律やベストプラクティスを明らかにすることができる。これらのルールの多くは、セキュリティ、保守性、信頼性に焦点を当てている。その中には、不適切な非同期使用、リソースリーク、コード全体の一貫性のないパターンなど、従来の静的解析ツール(SAST)では発見できない問題を扱うものもある。これらは、人間のレビュアーやAIを搭載したツールがコードレビュー中に発見できる種類の問題である。

課題

このような大規模プロジェクトでは、膨大なコード量、多くのモジュール(API、UI、プラグイン)、無数の外部統合(Prometheus、Lokiなど)といった課題に直面します。 何百人もの貢献者が異なるコーディングスタイルや前提に従うかもしれません。 新機能や迅速な修正によって、隠れたバグやセキュリティ上の欠陥、あるいは混乱したコードパスが発生する可能性があります。 ボランティアのレビュアーは、コードベースのすべての部分を知っているわけではないので、デザインパターンやベストプラクティスの見落としにつながるかもしれません。要するに、貢献の規模と多様性が、一貫性と信頼性を強制するのを難しくしているのだ。

なぜこのルールが重要なのか

明確な一連のレビュールールは、Grafanaの健全性に直接利益をもたらす。まず、保守性が向上します。一貫したパターン(フォルダのレイアウト、命名、エラー処理)によって、コードが読みやすくなり、テストしやすくなり、拡張しやすくなります。全員が共通の慣例に従っていれば、レビュアーが意図を推測するのに費やす時間は減る。第二に、セキュリティが強化される。「常にユーザー入力を検証する」や「オープンリダイレクトを避ける」といったルールが、Grafana .NETで発見された脆弱性(CVE-2025-6023/4123など)を防ぐ。最後に、新しい貢献者のオンボーディングはより速くなります:例とレビューが一貫して同じプラクティスを使用している場合、新人は「Grafanaの方法」を迅速かつ自信を持って学ぶことができます。

これらのルールへの架け橋となる文脈

これらのルールは、Grafanaのコードとコミュニティにおける実際の問題から来ています。 セキュリティ勧告やバグ報告によってパターン(XSSにつながるパストラバーサルなど)が発見され、それを予防的なルールに変えています。以下の各ルールは、具体的な落とし穴を強調し、なぜそれが重要なのか(パフォーマンス、明快さ、セキュリティなど)を説明し、Grafanaの言語(GoまたはTypeScript/JS)で❌非対✅準拠のスニペットを明確に示しています。

それでは、Grafanaのコードベースを堅牢で、安全で、理解しやすいものに保つための10のルールを探ってみよう。

Grafanaからヒントを得た実践的なコード・クオリティ・ルール10選

1.設定には環境変数を使う(ハードコードされた値は避ける)。

ポート、認証情報、URL、その他の環境固有の値のハードコーディングは避ける。環境変数や設定ファイルからそれらを読み取ることで、コードを柔軟に保ち、ソースから秘密を漏らさないようにする。

非準拠:

// server.js
constappPort = 3000;
app.listen(appPort、 () => コンソール.log("ポート "+ appPort));

準拠:

// server.ts
const PORT = Number(process.env.PORT) || 3000;
app.listen(PORT, () => console.log(`Listening on port ${PORT}`));

なぜこれが重要なのか:環境変数を使うことで、機密データをソースコードから排除し、異なる環境でも柔軟にデプロイできるようにし、機密の偶発的な漏洩を防ぎます。また、環境変数を使うことで、設定を変更してもコードを修正する必要がなくなり、保守性が向上し、エラーが減ります。

2.ユーザー入力を使用する前にサニタイズする。

インジェクション攻撃や予期せぬ動作を防ぐため、ユーザーや外部ソースからの入力はすべて、使用前に検証またはサニタイズされるべきである。

非準拠:

// frontend/src/components/UserForm.tsx
const handleSubmit = (username: string) => {
  setUsers([...users, { name: username }]);
};

準拠:

// frontend/src/utils/sanitize.ts
export function sanitizeInput(input: string): string {
  return input.replace(/<[^>]*>/g, ''); // removes HTML tags
}

// frontend/src/components/UserForm.tsx
import { sanitizeInput } from '../utils/sanitize';

const handleSubmit = (username: string) => {
  const cleanName = sanitizeInput(username);
  setUsers([...users, { name: cleanName }]);
};

なぜこれが重要なのか適切な入力サニタイズは、XSS、インジェクション攻撃、不正な入力による予期せぬ動作を防ぎます。これはユーザーとシステムの両方を保護し、下流のプロセス、ロギング、ストレージがデータを安全に扱うことを保証します。

3.オープンリダイレクトとパストラバーサルを防ぐ。

コードで使用される URL やファイルパスが適切に検証され、サニタイズされていることを確認してください。リダイレクトやファイルシステムのパスをユーザの入力で直接決定させないようにしてください。

非準拠:

// Express route in Grafana plugin
app.get("/goto", (req, res) => {
  const dest = req.query.next;    // attacker can supply any URL
  res.redirect(dest);
});

準拠:

// Express route with safe redirect
app.get("/goto", (req, res) => {
  const dest = req.query.next;
  // Only allow relative paths starting with '/'
  if (dest && dest.startsWith("/")) {
    res.redirect(dest);
  } else {
    res.status(400).send("Invalid redirect URL");
  }
});

なぜ重要なのかオープンリダイレクトとパストラバーサルを防止することで、フィッシング、データ漏洩、不正なファイルアクセスからユーザーを保護します。攻撃対象領域を減らし、セキュリティ境界を強化し、機密性の高いサーバー・リソースの偶発的な露出を回避します。

4.厳格なコンテンツ・セキュリティ・ポリシー(CSP)を有効にする。

信頼できるソースからのスクリプト、スタイル、画像、その他のリソースのみを許可するコンテンツセキュリティポリシーを、アプリケーションヘッダに強制する。安全でないインライン、eval、ワイルドカードのソースを許可しない。

非準拠:(CSP がない、または寛容すぎる)

# grafana.ini (非準拠)
content_security_policy = false

準拠:(強力な CSP を設定)

# grafana.ini
content_security_policy = true
content_security_policy_template = """
  script-src 'self' 'unsafe-eval' 'unsafe-inline' 'strict-dynamic' $NONCE
 object-src 'none';
 font-src 'self';
 style-src 'self' 'unsafe-inline' blob:;
 img-src * data:;
 base-uri 'self';
  connect-src 'self' grafana.com ws://$ROOT_PATH wss://$ROOT_PATH
 manifest-src 'self';
 media-src 'none';
 form-action 'self';
"""

なぜこれが重要なのか:厳格なCSPは、XSSを含む多くのクライアントサイド攻撃をブロックします。リソースの予測可能な動作を強制し、悪意のあるコードが実行される可能性を減らし、ブラウザ・コンテキストに明確なセキュリティ境界を提供します。

5.エラーとnilチェックの処理(パニックを避ける)。

関数呼び出し、API応答、データ構造内のエラーやnil値を常にチェックする。パニックを適切なエラー処理に置き換え、意味のあるエラーメッセージやコードを返す。

非準拠:

rows, _ := db.Query("SELECT * FROM users WHERE id=?", id)  // ignored error
user := &User{}
rows.Next()
rows.Scan(&user.Name)  // rows might be empty => user is nil => panic

準拠:

rows, err := db.Query("SELECT * FROM users WHERE id=?", id)
if err != nil {
    return nil, err
}
defer rows.Close()
if !rows.Next() {
    return nil, errors.New("user not found")
}
var name string
if err := rows.Scan(&name); err != nil {
    return nil, err
}
user := &User{Name: name}

なぜこれが重要なのか:適切なエラー処理はクラッシュを防ぎ、予期せぬ入力や条件が発生した場合でもシステムの信頼性を維持します。保守性を向上させ、ダウンタイムを減らし、意味のあるエラー情報を提供することでデバッグを容易にします。

6.資源の浄化を延期する(漏出を防ぐ)。

ファイル、ネットワーク接続、データベース・ハンドルなど、オープンされているすべてのリソースが、割り当て直後にdeferを使用して適切にクローズされていることを確認してください。コードの後半で手作業によるクリーンアップに頼らないでください。

非準拠:

resp, err := http.Get(url)
// ... resp.Bodyを使う ...
// 忘れられた: resp.Body.Close()

準拠:

resp, err := http.Get(url)
if err != nil {
    // handle error
}
defer resp.Body.Close()
// ... use resp.Body ...

なぜこれが重要なのか:適切なクリーンアップは、メモリー・リーク、ファイル記述子の枯渇、コネクション・プールの飽和を防ぎます。これにより、システムの安定性が維持され、時間の経過によるパフォーマンスの低下が回避され、本番環境での運用上の問題が軽減されます。

7.パラメータ化されたクエリを使用する(SQLインジェクションを避ける)。

データベースを操作する際には、SQLコマンドの文字列連結の代わりに、常にパラメータ化されたクエリまたはプリペアド・ステートメントを使用してください。

非準拠:

// 危険: userIDはSQL引用符またはインジェクションを含むかもしれない
query := "DELETE FROM sessions WHERE user_id = '"+ userID+"';"db.Exec(クエリ) + userID + "';"
db.Exec(query)

準拠:

// 安全: userIDがパラメータとして渡される
db.Exec("DELETE FROM sessions WHERE user_id = ?"userID)

なぜこれが重要なのか:パラメータ化されたクエリは、最も一般的なセキュリティ脆弱性の1つであるSQLインジェクション攻撃を防ぎます。パラメータ化されたクエリは、機密データを保護し、データベース破損のリスクを減らし、クエリをより保守しやすく、監査しやすくします。これにより、アプリケーションのセキュリティと信頼性の両方が保証されます。

8.TypeScriptでasync/awaitを適切に使う(約束を扱う)。

リジェクトを無視したり、コールバックスタイルの処理を混ぜるのではなく、︙つねに約束を待ち、try/catchを使ってエラーを処理する。

非準拠:

async function fetchData() {
  // Missing await: fetch returns a Promise, not the actual data
  const res = fetch('/api/values');
  console.log(res.data); // undefined
}

準拠:

async function fetchData() {
  try {
    const res = await fetch('/api/values');
    const data = await res.json();
    console.log(data);
  } catch (err) {
    console.error("Fetch failed:", err);
  }
}

なぜこれが重要なのか:適切な非同期処理を行うことで、非同期コードのエラーが気づかれないようにし、未処理のプロミス拒否を防ぎ、予測可能なプログラムフローを維持します。コードの可読性を高め、デバッグを容易にし、データの破損や一貫性のない状態、予期せぬランタイム・クラッシュにつながる微妙なバグを防ぎます。

9.9.TypeScriptではストリクトな型が好まれる。

変数、関数のパラメータ、戻り値の型を定義するには、anyの代わりに正確なTypeScriptの型を使用する。

非準拠:

// No types specified
function updateUser(data) {
  // ...
}
let config: any = loadConfig();

準拠:

interface User { id: number; name: string; }
function updateUser(data: User): Promise<User> {
  // ...
}
interface AppConfig { endpoint: string; timeoutMs: number; }
const config: AppConfig = loadConfig();

なぜこれが重要なのか:厳密な型付けは、コンパイル時に型に関連するミスを検出し、実行時のエラーを減らし、コードの信頼性を向上させます。それはコードを自己文書化し、リファクタリングを容易にし、システムのすべての部分が予測可能で型安全な方法で相互作用することを保証します。

10.一貫したコードスタイルと命名を適用する。

コードベース全体で統一されたフォーマット、命名規則、ファイル構造を強制する。

非準拠:(ミックススタイル)

const ApiData = await getdata();   // PascalCase for variable? function name not camelCase.
function Fetch_User() { ... }      // Unusual naming.

準拠:

const apiData = await fetchData();
function fetchUser() { ... }

なぜこれが重要なのか:一貫したスタイルとネーミングは可読性を向上させ、複数の貢献者がコードを理解し、維持することを容易にする。プロジェクトをナビゲートする際の認知的なオーバーヘッドを減らし、誤解による微妙なバグを防ぎ、自動化されたツール(リンター、フォーマッター、コード・レビュアー)が大規模なチーム環境で品質基準を確実に実施できるようにします。

結論

上記の各ルールは、Grafanaのコードベースで繰り返し発生する課題に対応しています。コードレビュー中に一貫して適用することで、チームはクリーンで予測可能なコードを維持し、一般的な脆弱性を防ぐことでセキュリティを向上させ、新しい貢献者に明確なパターンを提供することでオンボーディングをスムーズにすることができます。プロジェクトの規模が拡大するにつれて、これらのプラクティスは、コードベースの信頼性、保守性を維持し、関係者全員が簡単に操作できるようにする。これらのルールに従うことで、どのようなエンジニアリングチームでも、高品質なソフトウェアを大規模に構築し、維持することができる。

よくある質問

ご質問は?

なぜGrafanaのリポジトリでコードレビューのルールを分析するのか?

Grafanaは、何千人もの貢献者を擁する大規模で成熟したオープンソースプロジェクトである。そのコードベースを研究することで、チームがクリーンでスケーラブルかつセキュアなソフトウェアを大規模に維持するのに役立つ実世界のエンジニアリングパターンが明らかになる。

これらのルールは、通常のリンティングや書式チェックと何が違うのか?

従来のリンターは、構文やフォーマットの問題を捉えます。これらのGrafanaベースのルールは、より深く、アーキテクチャ、可読性、一貫性、およびAIベースのレビューが扱うことができる文脈の理解を必要とするセキュリティ上の決定に焦点を当てる。

AIベースのツールは、このようなコード品質の問題の検出にどのように役立つのだろうか?

AIツールは、構文だけでなく、意図、ネーミング、アーキテクチャ、コンテキストを分析できる。従来の静的解析では見逃されがちな、保守性の問題や不明確な抽象化、潜在的なセキュリティ問題を特定することができる。

これらのルールはGrafana特有のものですか、それともどのチームでも使えるのですか?

これらはGrafanaのコードベースにインスパイアされていますが、原則はどのような大規模ソフトウェアプロジェクトにも当てはまります。チームは、一貫性を維持し、リグレッションを防止し、オンボーディングを改善するために、独自のリポジトリに適応させることができます。

これらのルールは、Aikidoコード品質チェックとどのように関係しているのだろうか?

各ルールは、Aikidoコード・クオリティ・プラットフォームのカスタムAIルールとして実装することができ、すべてのプル・リクエストにおいて、アーキテクチャ、可読性、セキュリティの問題を自動検出することができる。

まずは無料で体験

コード、クラウド、ランタイムを1つの中央システムでセキュアに。
脆弱性を迅速に発見し、自動的に修正。

クレジットカードは不要。